Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bugfix: Support serialization for dm.jdbc.driver.DmdbTimestamp (#6701) #6728

Merged
merged 5 commits into from
Aug 11, 2024

Conversation

lyl2008dsg
Copy link
Contributor

@lyl2008dsg lyl2008dsg commented Aug 3, 2024

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

This pull request introduces support for serialization and deserialization of the dm.jdbc.driver.DmdbTimestamp class. The changes ensure that environments lacking the DM JDBC driver do not encounter errors or unnecessary logging.

Changes Made

  1. Conditional Registration of Serializers and Deserializers:
  • Added logic to register serializers and deserializers for dm.jdbc.driver.DmdbTimestamp only if the class is present in the classpath.
  • This avoids issues in environments where the DM JDBC driver is not available.
  1. Error Handling:
  • No error logs are recorded if the dm.jdbc.driver.DmdbTimestamp class is not found.
  • This is to prevent confusion for users who do not have the DM JDBC driver installed.

Ⅱ. Does this pull request fix one issue?

fixes #6701

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@funky-eyes funky-eyes added type: bug Category issues or prs related to bug. first-time contributor first-time contributor module/rm-datasource rm-datasource module labels Aug 3, 2024
@funky-eyes funky-eyes added this to the 2.2.0 milestone Aug 3, 2024
Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

我认为你应该需要在rm-datasource中引入以下依赖
I think you should need to introduce the following dependencies in rm-datasource

            <dependency>
                <groupId>com.dameng</groupId>
                <artifactId>DmJdbcDriver18</artifactId>
                <scope>test</scope>
            </dependency>

@lyl2008dsg
Copy link
Contributor Author

我认为你应该需要在rm-datasource中引入以下依赖 I think you should need to introduce the following dependencies in rm-datasource

            <dependency>
                <groupId>com.dameng</groupId>
                <artifactId>DmJdbcDriver18</artifactId>
                <scope>test</scope>
            </dependency>

done.
Yes, I think so too. It can improve unit test coverage.

Copy link

codecov bot commented Aug 3, 2024

Codecov Report

Attention: Patch coverage is 76.36364% with 13 lines in your changes missing coverage. Please review.

Project coverage is 50.88%. Comparing base (005ac0b) to head (dca4610).
Report is 1 commits behind head on 2.x.

Files Patch % Lines
...m/datasource/undo/parser/JacksonUndoLogParser.java 76.36% 12 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6728      +/-   ##
============================================
+ Coverage     50.83%   50.88%   +0.04%     
- Complexity     5913     5914       +1     
============================================
  Files          1058     1058              
  Lines         36717    36772      +55     
  Branches       4362     4368       +6     
============================================
+ Hits          18666    18710      +44     
- Misses        16162    16173      +11     
  Partials       1889     1889              
Files Coverage Δ
...m/datasource/undo/parser/JacksonUndoLogParser.java 71.06% <76.36%> (+2.05%) ⬆️

... and 3 files with indirect coverage changes

Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@xingfudeshi xingfudeshi left a comment

Choose a reason for hiding this comment

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

LGTM

@funky-eyes funky-eyes merged commit 35820c0 into apache:2.x Aug 11, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time contributor first-time contributor module/rm-datasource rm-datasource module type: bug Category issues or prs related to bug.
Projects
None yet
3 participants