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

feat(be+fe): Features for FUTU #590

Merged
merged 148 commits into from
Mar 29, 2024
Merged

feat(be+fe): Features for FUTU #590

merged 148 commits into from
Mar 29, 2024

Conversation

530051970
Copy link
Contributor

**What is the current behavior?

Please describe the current behavior of the code that you are trying to modify. This could include any issues or limitations that you are addressing.

**What is the updated behavior?

Please describe the changes you are making to the code's behavior. This could include new features or enhancements that you are adding, as well as how they differ from the current behavior.

**Checklist

Please check off the following items before submitting your pull request:

  • I have tested my changes.
  • No sensitive information is included.
  • I've reviewed my changes to ensure they won't cause any new issues that could affect the stability or performance of the codebase.

Please check off the following items if this changes include API modefications:

  • I confirm this changes has been test with authorization validation steps, it won't break the authorization token verification mechanism.
  • I confirm this changes won't cause security issues.

530051970 and others added 30 commits January 19, 2024 11:01
feat(fe): add batch upload file and system settings
feat(be): Always update the schema of MySQL
fix(cdk): add batch create jdbc datasource template
fix(cdk): add batch create jdbc datasource template
@530051970 530051970 requested a review from yanbasic March 27, 2024 08:20
@530051970 530051970 requested a review from a team as a code owner March 27, 2024 08:20
yanbasic
yanbasic previously approved these changes Mar 27, 2024
Copy link
Contributor

@yanbasic yanbasic left a comment

Choose a reason for hiding this comment

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

LGTM

@530051970 530051970 changed the title feat: Features for FUTU feat(be+fe): Features for FUTU Mar 28, 2024
nowfox
nowfox previously approved these changes Mar 29, 2024
@nowfox nowfox merged commit 9e612b7 into main Mar 29, 2024
12 checks passed
)
return url

def batch_create(file: UploadFile = File(...)):
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

The cyclomatic complexity of this function is 32. By comparison, 99% of the functions in the CodeGuru reference dataset have a lower cyclomatic complexity. This indicates the function has a high number of decisions and it can make the logic difficult to understand and test.

We recommend that you simplify this function or break it into multiple functions. For example, consider extracting the code block on lines 339-349 into a separate function.

# logger.info(databases)
# return databases

def batch_create(file: UploadFile = File(...)):
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

This function contains 65 lines of code, not including blank lines or lines with only comments, Python punctuation characters, identifiers, or literals. By comparison, 99% of the functions in the CodeGuru reference dataset contain fewer lines of code. Large functions might be difficult to read and have logic that is hard to understand and test.

We recommend that you simplify this function or break it into multiple functions. For example, consider extracting the code blocks on lines 2735-2773, lines 2775-2793 into separate functions.

@@ -730,9 +749,14 @@

return True

def delete_jdbc_connection(provider_id: int, account: str, region: str, instance_id: str, delete_catalog_only=False):
async def __delete_jdbc_connection(provider_id: int, account: str, region: str, instance_id: str, delete_catalog_only=False):
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

This function contains 62 lines of code, not including blank lines or lines with only comments, Python punctuation characters, identifiers, or literals. By comparison, 99% of the functions in the CodeGuru reference dataset contain fewer lines of code. Large functions might be difficult to read and have logic that is hard to understand and test.

We recommend that you simplify this function or break it into multiple functions. For example, consider extracting the code block on lines 754-759 into a separate function.

async def __add_jdbc_conn_batch(jdbc: JDBCInstanceSource):
try:
add_jdbc_conn(jdbc)
return jdbc.account_provider_id, jdbc.account_id, jdbc.region, jdbc.instance_id, "SUCCESSED", None
Copy link
Contributor

Choose a reason for hiding this comment

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

Recommendation generated by Amazon CodeGuru Reviewer. Leave feedback on this recommendation by replying to the comment or by reacting to the comment using emoji.

It appears you are unpacking more than three variables while using a function that returns more than one variable. Using a large number of return values is prone to errors. We recommend that you return a small class or namedtuple instance instead.

Learn more

Similar issue at line numbers 2954 and 2957.

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.

None yet

5 participants