-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Fix the release retention error #5162
Fix the release retention error #5162
Conversation
WalkthroughIn version 2.3.0 of Apollo, a critical fix was implemented to resolve SQL errors associated with the configuration retention feature in MySQL. This involved updating SQL delete statements in entity classes to properly escape table names using backticks (`). Changes
Assessment against linked issues
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
CHANGES.md (3)
16-16
: Add a blank line above this entry to adhere to markdown best practices.* [Fix the error occurred when using configuration retention feature](https://github.com/apolloconfig/apollo/pull/5162)
Line range hint
1-1
: Surround headings with blank lines for better markdown formatting.Changes by Version
Line range hint
8-8
: Surround the list of changes with blank lines to meet markdown formatting standards.* [Fix circular references on LdapAutoConfiguration](https://github.com/apolloconfig/apollo/pull/5055)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (23)
- CHANGES.md (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/AccessKey.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Audit.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Cluster.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Commit.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/GrayReleaseRule.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Item.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Namespace.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Privilege.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Release.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/ReleaseHistory.java (1 hunks)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/ServerConfig.java (1 hunks)
- apollo-common/src/main/java/com/ctrip/framework/apollo/common/entity/App.java (1 hunks)
- apollo-common/src/main/java/com/ctrip/framework/apollo/common/entity/AppNamespace.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/Consumer.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/ConsumerRole.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/ConsumerToken.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Favorite.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Permission.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Role.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/RolePermission.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/ServerConfig.java (1 hunks)
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/UserRole.java (1 hunks)
Files skipped from review due to trivial changes (12)
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/AccessKey.java
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Audit.java
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Namespace.java
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/ReleaseHistory.java
- apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/ServerConfig.java
- apollo-common/src/main/java/com/ctrip/framework/apollo/common/entity/AppNamespace.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/ConsumerRole.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Favorite.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Permission.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/Role.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/RolePermission.java
- apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/UserRole.java
Additional context used
Markdownlint
CHANGES.md
1-1: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines
8-8: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines
Additional comments not posted (10)
apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Privilege.java (1)
30-30
: Ensure theIsDeleted
flag andDeletedAt
timestamp are updated correctly in your business logic to reflect these changes.apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/ConsumerToken.java (1)
35-35
: This change aligns with the overall strategy of using backticks for table names. Good job ensuring SQL compatibility.apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Cluster.java (1)
33-33
: The updated SQLDelete annotation correctly uses backticks and follows the new deletion logic pattern. This change ensures consistency across your entity classes.apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/entity/po/ServerConfig.java (1)
36-36
: The use of backticks around the table name in the SQLDelete annotation is a good practice for SQL compatibility, especially for MySQL.apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Commit.java (1)
31-31
: Escaping the table name in the SQLDelete statement helps prevent SQL syntax errors in environments like MySQL.apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Item.java (1)
31-31
: Good practice to escape table names in SQL statements to avoid potential SQL syntax issues.apollo-portal/src/main/java/com/ctrip/framework/apollo/openapi/entity/Consumer.java (1)
30-30
: Correct use of backticks in the SQLDelete statement to ensure SQL syntax compatibility across different database systems.apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/GrayReleaseRule.java (1)
30-30
: Correctly updated the SQLDelete statement to escape the table name, which should prevent potential SQL syntax issues.apollo-biz/src/main/java/com/ctrip/framework/apollo/biz/entity/Release.java (1)
34-34
: Correctly updated the SQLDelete statement to escape the table nameRelease
, which is crucial given that "Release" could be a reserved SQL keyword.apollo-common/src/main/java/com/ctrip/framework/apollo/common/entity/App.java (1)
33-33
: Correctly updated the SQLDelete statement to escape the table nameApp
, enhancing SQL syntax safety.
Thanks for this fix |
What's the purpose of this PR
Fix the release retention error
Which issue(s) this PR fixes:
Fixes #5161
Brief changelog
@SQLDelete
statementsFollow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.CHANGES
log.Summary by CodeRabbit
Bug Fixes
Refactor