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

optimize: compatible with lower versions of SPI #6315

Merged
merged 19 commits into from
Feb 19, 2024

Conversation

xingfudeshi
Copy link
Member

@xingfudeshi xingfudeshi commented Jan 30, 2024

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

fixes #6307

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-commenter
Copy link

codecov-commenter commented Jan 30, 2024

Codecov Report

Attention: 57 lines in your changes are missing coverage. Please review.

Comparison is base (4006df1) 51.93% compared to head (f86bb9a) 52.39%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6315      +/-   ##
============================================
+ Coverage     51.93%   52.39%   +0.45%     
- Complexity     5185     5216      +31     
============================================
  Files           921      920       -1     
  Lines         32166    31913     -253     
  Branches       3874     3805      -69     
============================================
+ Hits          16706    16721      +15     
+ Misses        13822    13546     -276     
- Partials       1638     1646       +8     
Files Coverage Δ
...che/seata/common/loader/EnhancedServiceLoader.java 60.00% <24.00%> (-11.99%) ⬇️

... and 12 files with indirect coverage changes

@xingfudeshi xingfudeshi changed the title optimize:compatible with lower versions of SPI (WIP)optimize:compatible with lower versions of SPI Jan 30, 2024
@xingfudeshi xingfudeshi changed the title (WIP)optimize:compatible with lower versions of SPI optimize:compatible with lower versions of SPI Feb 4, 2024
compatible/pom.xml Outdated Show resolved Hide resolved
@xingfudeshi xingfudeshi changed the title optimize:compatible with lower versions of SPI (WIP)optimize:compatible with lower versions of SPI Feb 6, 2024
@xingfudeshi xingfudeshi changed the title (WIP)optimize:compatible with lower versions of SPI optimize:compatible with lower versions of SPI Feb 7, 2024
Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

I suggest changing the SPI loading priority here.

  1. Load the Apache Seata SPI first. Because Apache Seata is the final state of the future.
  2. If the Apache seata SPI cannot be found, output a warn log and try to load the io.seata SPI.
  3. If the io.seata SPI also fails to be loaded, handle the exception according to the logic in the original SPI.

@xingfudeshi
Copy link
Member Author

I suggest changing the SPI loading priority here.

  1. Load the Apache Seata SPI first. Because Apache Seata is the final state of the future.
  2. If the Apache seata SPI cannot be found, output a warn log and try to load the io.seata SPI.
  3. If the io.seata SPI also fails to be loaded, handle the exception according to the logic in the original SPI.

Ok

@xingfudeshi
Copy link
Member Author

Done. @slievrly

Copy link
Member

@slievrly slievrly left a comment

Choose a reason for hiding this comment

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

LGTM

@slievrly slievrly changed the title optimize:compatible with lower versions of SPI optimize: compatible with lower versions of SPI Feb 19, 2024
@slievrly slievrly merged commit 5667d63 into apache:2.x Feb 19, 2024
6 checks passed
@slievrly slievrly added this to the 2.1.0 milestone Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compatible with lower versions of SPI
3 participants