Skip to content

Conversation

@linjianchang
Copy link

[AMORO-3989] Amoro's support for the domestic database Damang

Copy link
Contributor

@xxubai xxubai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you fix the unit tests

@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2025

Codecov Report

❌ Patch coverage is 36.36364% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 7.80%. Comparing base (cbdc517) to head (6b2aa2d).
⚠️ Report is 15 commits behind head on master.

Files with missing lines Patch % Lines
...he/amoro/server/persistence/DataSourceFactory.java 0.00% 5 Missing and 2 partials ⚠️

❗ There is a different number of reports uploaded between BASE (cbdc517) and HEAD (6b2aa2d). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (cbdc517) HEAD (6b2aa2d)
trino 1 0
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #3992       +/-   ##
============================================
- Coverage     22.12%   7.80%   -14.33%     
+ Complexity     2461     895     -1566     
============================================
  Files           445     639      +194     
  Lines         40897   51508    +10611     
  Branches       5767    6545      +778     
============================================
- Hits           9050    4019     -5031     
- Misses        31089   47233    +16144     
+ Partials        758     256      -502     
Flag Coverage Δ
core 7.80% <36.36%> (?)
trino ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

<dependency>
<groupId>com.dameng</groupId>
<artifactId>DmJdbcDriver18</artifactId>
</dependency>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be provided

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be provided

企业微信截图_17659411772400 I see that the JDBC driver packages for PostgreSQL and Derby are not provided

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think Dameng needs to be included in the release. Similar to MySQL, users who need it can add the JDBC dependency manually, rather than having it as a standard component.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think Dameng needs to be included in the release. Similar to MySQL, users who need it can add the JDBC dependency manually, rather than having it as a standard component.

Aleady modified!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xxubai Thanks for your review. Do you have any other suggestions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants