-
Notifications
You must be signed in to change notification settings - Fork 9
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
Conversation
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
source/constructs/lib/agent/delete-agent-resources/delete_agent_resources.py
Show resolved
Hide resolved
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.
LGTM
) | ||
return url | ||
|
||
def batch_create(file: UploadFile = 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.
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(...)): |
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.
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): |
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.
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 |
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.
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.
Similar issue at line numbers 2954 and 2957.
**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:
Please check off the following items if this changes include API modefications: