-
Notifications
You must be signed in to change notification settings - Fork 5k
merge: from 3.3.6 to main branch #33817
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
base: main
Are you sure you want to change the base?
Conversation
* fix(log): replace distutils.log with warnings for logging * fix(logging): replace warnings.warn with print in TDLog class * fix(log): replace print with warnings.warn in success method * fix(tests): update grep command to include binary files for success check * fix(tests): add space in grep command for consistency in success check --------- Co-authored-by: WANG Xu <[email protected]>
Summary of ChangesHello @guanshengliang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates a series of updates from the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request merges changes from the 3.3.6 release branch into main. The changes are diverse, including schema updates for system tables, logic adjustments for vgroup readiness and sync pipeline restoration, and cleanup of deprecated Python imports. Additionally, a new test case has been added and a test script has been updated.
My review has identified a potential logic bug in how a vgroup's "ready" state is determined, which could affect single-replica vgroups. I've also noted a possible typo in the path of a newly added test case in the CI configuration. Please see the detailed comments for suggestions.
| } | ||
| } | ||
|
|
||
| if (isLeaderRestored && hasFollowerRestored) isReady = true; |
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.
The logic for determining if a vgroup is ready appears incorrect for single-replica vgroups. When pVgroup->replica == 1, hasFollowerRestored will always be false, causing isReady to be false even if the leader is restored. A single-replica vgroup should be considered ready if its only member (the leader) is restored.
isReady = isLeaderRestored && (pVgroup->replica == 1 || hasFollowerRestored);| ## 04-OrderBy | ||
| ,,y,.,./ci/pytest.sh pytest cases/09-DataQuerying/04-OrderBy/test_orderby_double.py | ||
| ,,y,.,./ci/pytest.sh pytest cases/09-DataQuerying/04-OrderBy/test_orderby_subquery.py | ||
| ,,y,.,./ci/pytest.sh pytest cases/07-DataQuerying/04-OrderBy/test_order_by_select_list.py |
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.
The path for the newly added test case, cases/07-DataQuerying/04-OrderBy/test_order_by_select_list.py, seems to use a different directory number (07-DataQuerying) compared to the surrounding tests which are in 09-DataQuerying. This might be a typo. Please verify if the path is correct. If it's a typo, it should probably be cases/09-DataQuerying/04-OrderBy/test_order_by_select_list.py.
,,y,.,./ci/pytest.sh pytest cases/09-DataQuerying/04-OrderBy/test_order_by_select_list.py
Description
Please briefly describe the code changes in this pull request.
Jira: https://jira.taosdata.com:18080/browse/TD-
Checklist
Please check the items in the checklist if applicable.