Fixed pylint issue from common directory #3016
Conversation
|
Can one of the admins verify this patch? |
youngjae-hur7
force-pushed
the
fix_pylint_common
branch
from
5b129c8 to
53ad383
Compare
| def create_connection(): | ||
| """ | ||
| Create a database connection to the omniadb. | ||
|
|
||
| This function establishes a connection to the omniadb database using the provided password. | ||
| It reads the encrypted password from a file, decrypts it using the provided key, and connects to the database. | ||
| It reads the encrypted password from a file, | ||
| decrypts it using the provided key, and connects to the database. |
There was a problem hiding this comment.
Is there a reason in all the comment blocks there is an additional indent for longer lines? It reads a little weird to have an extra indent.
There was a problem hiding this comment.
I agree. pylint only gives us 100 spaces for each line. It's kinda a tradeoff beween whether we sacrifice readability or improve the pylint score, and I chose the later option
| @@ -160,7 +198,8 @@ def get_data_from_db(db='omniadb', table_name='cluster.nodeinfo', filter_dict={} | |||
| """ | |||
| conn = get_db_connection(db) | |||
| cursor = conn.cursor(cursor_factory=DictCursor) | |||
|
|
|||
| if filter_dict is None: | |||
There was a problem hiding this comment.
Why add this, instead of having it default to an empty dict like it originally was?
There was a problem hiding this comment.
yea unfortunately pylint complains about the filter_dict={} as an argument. so I've updated it as filter_dict=None and checks for the None case in the line 201 because as Suman commented above in this PR, there are some cases that filter_dict aren't None
| else: | ||
| message = f"Unsupported file: {input_file_path, data}" | ||
| logger.error(message) | ||
| # else: |
| @@ -284,65 +288,72 @@ def validate_vip_address( | |||
| service_node_vip, | |||
| admin_network, | |||
| admin_netmaskbits, | |||
| oim_admin_ip | |||
| oim_admin_ip, | |||
There was a problem hiding this comment.
There seems to be a trailing comma for all functions in this file?
There was a problem hiding this comment.
what a hawk eyes! thanks Coleman! fixed
| groups[group].get(parent, None) | ||
| ): | ||
| # If parent is not empty and group is associated with login, | ||
| # compiler_node, service_node, kube_control_plane, |
|
run omnia-checkers |
|
run omnia-checkers |
Issues Resolved by this Pull Request
Please be sure to associate your pull request with one or more open issues. Use the word Fixes as well as a hashtag (#) prior to the issue number in order to automatically resolve associated issues (e.g., Fixes #100).
Fixes #
Description of the Solution
Please describe the solution provided and how it resolves the associated issues.
Suggested Reviewers
If you wish to suggest specific reviewers for this solution, please include them in this section. Be sure to include the @ before the GitHub username.
@abhishek-sa1
@priti-parate