-
Notifications
You must be signed in to change notification settings - Fork 82
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
Ruff code auto-format #1105
Ruff code auto-format #1105
Changes from all commits
8e65494
3b66891
2b665b0
9cf83e0
db5a608
e3374c6
fa58cf1
f762a8a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -70,3 +70,4 @@ npm-debug.log* | |
yarn-debug.log* | ||
yarn-error.log* | ||
.idea | ||
/.ruff_cache/ |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,8 +34,9 @@ install-tests: | |
pip install -r tests/requirements.txt | ||
|
||
lint: | ||
pip install flake8 | ||
python -m flake8 --exclude cdk.out,blueprints --ignore E402,E501,F841,W503,F405,F403,F401,E712,E203 backend/ | ||
pip install ruff | ||
ruff check --fix | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't we run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. btw forgot to say that we should be taking into account both check and format exit codes and fail the target if either of them fail. |
||
ruff format | ||
|
||
bandit: | ||
pip install bandit | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,6 @@ | |
|
||
|
||
class Cognito(ServiceProvider): | ||
|
||
def __init__(self): | ||
self.client = boto3.client('cognito-idp', region_name=os.getenv('AWS_REGION', 'eu-west-1')) | ||
|
||
|
@@ -19,10 +18,7 @@ def get_user_emailids_from_group(self, groupName): | |
ssm = boto3.client('ssm', region_name=os.getenv('AWS_REGION', 'eu-west-1')) | ||
user_pool_id = ssm.get_parameter(Name=parameter_path)['Parameter']['Value'] | ||
paginator = self.client.get_paginator('list_users_in_group') | ||
pages = paginator.paginate( | ||
UserPoolId=user_pool_id, | ||
GroupName=groupName | ||
) | ||
pages = paginator.paginate(UserPoolId=user_pool_id, GroupName=groupName) | ||
cognito_user_list = [] | ||
for page in pages: | ||
cognito_user_list += page['Users'] | ||
|
@@ -38,9 +34,7 @@ def get_user_emailids_from_group(self, groupName): | |
if envname in ['local', 'dkrcompose']: | ||
log.error('Local development environment does not support Cognito') | ||
return ['[email protected]'] | ||
log.error( | ||
f'Failed to get email ids for Cognito group {groupName} due to {e}' | ||
) | ||
log.error(f'Failed to get email ids for Cognito group {groupName} due to {e}') | ||
raise e | ||
else: | ||
return group_email_ids | ||
|
@@ -58,9 +52,7 @@ def list_groups(self, envname: str, region: str): | |
for page in pages: | ||
groups += [gr['GroupName'] for gr in page['Groups']] | ||
except Exception as e: | ||
log.error( | ||
f'Failed to list groups of user pool {user_pool_id} due to {e}' | ||
) | ||
log.error(f'Failed to list groups of user pool {user_pool_id} due to {e}') | ||
raise e | ||
return groups | ||
|
||
|
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.
I wouldn't makefile target tool specific, let's keep it as
lint
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.
May be we can keep both? I don't want the name to be confusing, since it's just an alias
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.
Previously we were running
flake8
and we called the targetlint
and notflake8
which I think is correct because it's abstracting whatever underlying linter we use and also that way we wouldn't have to change the pipelines code.