-
Notifications
You must be signed in to change notification settings - Fork 0
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
Sourcery refactored tuning branch #1
base: tuning
Are you sure you want to change the base?
Conversation
parameters = dict( | ||
(k, expand_path(v)) | ||
for k, v in args(params_args) | ||
) | ||
parameters = {k: expand_path(v) for k, v in args(params_args)} |
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.
Lines 24-27
refactored with the following changes:
- Replace list(), dict() or set() with comprehension (
collection-builtin-to-comprehension
)
r = requests.get('https://www.googleapis.com/oauth2/v3/tokeninfo?access_token=' + token) | ||
r = requests.get( | ||
f'https://www.googleapis.com/oauth2/v3/tokeninfo?access_token={token}' | ||
) |
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.
Function radlabauth
refactored with the following changes:
- Use f-string instead of string concatenation [×3] (
use-fstring-for-concatenation
)
elif (select_proj != '1' and select_proj != '2'): | ||
elif select_proj != '1': | ||
sys.exit(Fore.RED + "\nError Occured - INVALID choice.\n") | ||
else: | ||
projid = input(Fore.YELLOW + Style.BRIGHT + "\nEnter the Project ID for RAD Lab management" + Style.RESET_ALL + ': ').strip() | ||
else: | ||
pass | ||
|
||
os.system("gcloud config set project " + projid) | ||
os.system("gcloud auth application-default set-quota-project " + projid ) | ||
os.system(f"gcloud config set project {projid}") | ||
os.system(f"gcloud auth application-default set-quota-project {projid}") |
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.
Function set_proj
refactored with the following changes:
- Remove redundant conditional (
remove-redundant-if
) - Remove redundant pass statement (
remove-redundant-pass
) - Use f-string instead of string concatenation [×2] (
use-fstring-for-concatenation
)
# Hardcoded Org level required RAD Lab Launcher roles | ||
launcherorgroles = ['roles/iam.organizationRoleViewer'] | ||
|
||
credentials = GoogleCredentials.get_application_default() | ||
|
||
service0 = discovery.build('cloudresourcemanager', 'v3', credentials=credentials) | ||
request0 = service0.projects().getIamPolicy(resource='projects/' + projid) | ||
request0 = service0.projects().getIamPolicy(resource=f'projects/{projid}') |
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.
Function launcherperm
refactored with the following changes:
- Move assignments closer to their usage (
move-assign
) - Use f-string instead of string concatenation [×5] (
use-fstring-for-concatenation
) - Remove redundant pass statement [×2] (
remove-redundant-pass
)
This removes the following comments ( why? ):
# Check for Required roles on RAD Lab Management Project
if 'folders' in parent: | ||
credentials = GoogleCredentials.get_application_default() | ||
s = discovery.build('cloudresourcemanager', 'v3', credentials=credentials) | ||
req = s.folders().get(name=parent) | ||
res = req.execute() | ||
return findorg(res['parent']) | ||
else: | ||
if 'folders' not in parent: | ||
# print(Fore.GREEN + "Org identified: " + Style.BRIGHT + parent + Style.RESET_ALL) | ||
return parent | ||
credentials = GoogleCredentials.get_application_default() | ||
s = discovery.build('cloudresourcemanager', 'v3', credentials=credentials) | ||
req = s.folders().get(name=parent) | ||
res = req.execute() | ||
return findorg(res['parent']) |
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.
Function findorg
refactored with the following changes:
- Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
print("----------> RUN FOR: " + dir) | ||
print(f"----------> RUN FOR: {dir}") | ||
|
||
|
||
try: | ||
# IF MODULE EXISTS: Copying main directory in temp folder | ||
shutil.copytree(GITHUB_WORKSPACE+'/'+dir, os.getcwd()+'/temp/'+dir) | ||
shutil.copytree(f'{GITHUB_WORKSPACE}/{dir}', f'{os.getcwd()}/temp/{dir}') | ||
|
||
# Deleting added/modified & removed files | ||
for mfile in modified_files: | ||
if os.path.exists(os.getcwd()+'/temp/'+mfile): | ||
print("Deleting file: " + mfile) | ||
os.remove(os.getcwd()+'/temp/'+mfile) | ||
if os.path.exists(f'{os.getcwd()}/temp/{mfile}'): | ||
print(f"Deleting file: {mfile}") | ||
os.remove(f'{os.getcwd()}/temp/{mfile}') | ||
|
||
for rfile in removed_files: | ||
if os.path.exists(os.getcwd()+'/temp/'+rfile): | ||
print("Deleting file: " + rfile) | ||
os.remove(os.getcwd()+'/temp/'+rfile) | ||
if os.path.exists(f'{os.getcwd()}/temp/{rfile}'): | ||
print(f"Deleting file: {rfile}") | ||
os.remove(f'{os.getcwd()}/temp/{rfile}') | ||
except: | ||
# IF MODULE DONOT EXISTS: Creating temp module folder | ||
os.makedirs(os.getcwd()+'/temp/'+dir) | ||
os.makedirs(f'{os.getcwd()}/temp/{dir}') |
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.
Function main
refactored with the following changes:
- Use f-string instead of string concatenation [×26] (
use-fstring-for-concatenation
)
response = requests.get('https://api.github.com/repos/'+ GITHUB_REPOSITORY +'/pulls/'+ str(pr) +'/files') | ||
response = requests.get( | ||
f'https://api.github.com/repos/{GITHUB_REPOSITORY}/pulls/{str(pr)}/files' | ||
) |
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.
Function pr_files
refactored with the following changes:
- Use f-string instead of string concatenation [×4] (
use-fstring-for-concatenation
)
# print(path) | ||
if not os.path.exists(path): | ||
os.makedirs(path) | ||
# print(path) | ||
if not os.path.exists(path): | ||
os.makedirs(path) | ||
|
||
# print('Beginning file download with requests') | ||
r = requests.get(raw) | ||
with open(path + '/' + os.path.basename(file), 'wb') as f: | ||
f.write(r.content) | ||
# print('Beginning file download with requests') | ||
r = requests.get(raw) | ||
with open(f'{path}/{os.path.basename(file)}', 'wb') as f: | ||
f.write(r.content) |
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.
Function downloadprfiles
refactored with the following changes:
- Use f-string instead of string concatenation [×2] (
use-fstring-for-concatenation
)
modified_files_dir = [] | ||
removed_files_dir = [] | ||
|
||
for file in modified_files: | ||
modified_files_dir.append(os.path.dirname(file)) | ||
|
||
for file in removed_files: | ||
removed_files_dir.append(os.path.dirname(file)) | ||
|
||
working_directories = modified_files_dir + removed_files_dir | ||
working_directories = list(set(working_directories)) | ||
modified_files_dir = [os.path.dirname(file) for file in modified_files] | ||
removed_files_dir = [os.path.dirname(file) for file in removed_files] | ||
working_directories = modified_files_dir + removed_files_dir | ||
working_directories = list(set(working_directories)) | ||
|
||
# print("Working Directories:") | ||
# print(working_directories) | ||
|
||
modules = [x for x in working_directories if x.startswith('modules/')] | ||
modules = [x for x in modules if x.count('/') == 1] | ||
print("Modules Updated:") | ||
print(modules) | ||
|
||
return modules | ||
modules = [x for x in working_directories if x.startswith('modules/')] | ||
modules = [x for x in modules if x.count('/') == 1] | ||
print("Modules Updated:") | ||
print(modules) | ||
|
||
return modules |
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.
Function get_updated_modules
refactored with the following changes:
- Move assignment closer to its usage within a block [×2] (
move-assign-in-block
) - Convert for loop into list comprehension [×2] (
list-comprehension
)
tr = Terraform(working_dir=dir) | ||
|
||
return_code_init, stdout_init, stderr_init = tr.init_cmd(capture_output=False) | ||
return_code_plan, stdout_plan, stderr_plan = tr.plan_cmd(capture_output=False,var={'billing_account_id':'ABCD-EFGH-IJKL-MNOP', 'organization_id':'1234567890', 'random_id': '1234'}) | ||
path = os.getcwd()+'/temp/' | ||
if(return_code_init == 1): | ||
comment = 'Terraform Init FAILED' | ||
status = 'fail' | ||
if(return_code_plan == 1): | ||
comment = 'Terraform Plan FAILED' | ||
status = 'fail' | ||
else: | ||
comment = 'Terraform Init & Terraform Plan SUCCESSFUL' | ||
status = 'pass' | ||
return comment, status | ||
tr = Terraform(working_dir=dir) | ||
|
||
return_code_init, stdout_init, stderr_init = tr.init_cmd(capture_output=False) | ||
return_code_plan, stdout_plan, stderr_plan = tr.plan_cmd(capture_output=False,var={'billing_account_id':'ABCD-EFGH-IJKL-MNOP', 'organization_id':'1234567890', 'random_id': '1234'}) | ||
|
||
path = f'{os.getcwd()}/temp/' | ||
if(return_code_init == 1): | ||
comment = 'Terraform Init FAILED' | ||
status = 'fail' | ||
if(return_code_plan == 1): | ||
comment = 'Terraform Plan FAILED' | ||
status = 'fail' | ||
else: | ||
comment = 'Terraform Init & Terraform Plan SUCCESSFUL' | ||
status = 'pass' | ||
|
||
return comment, status |
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.
Function tf
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
response = requests.post('https://api.github.com/repos/'+ GITHUB_REPOSITORY +'/issues/'+ str(pr) +'/comments', data=json.dumps(data), headers=headers) | ||
# print(response.text) | ||
response = requests.post( | ||
f'https://api.github.com/repos/{GITHUB_REPOSITORY}/issues/{str(pr)}/comments', | ||
data=json.dumps(data), | ||
headers=headers, | ||
) | ||
# print(response.text) |
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.
Function commentpr
refactored with the following changes:
- Use f-string instead of string concatenation [×4] (
use-fstring-for-concatenation
)
print('\n'.join(' - {}'.format(s) for s in warnings)) | ||
print('\n'.join(f' - {s}' for s in warnings)) |
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.
Function main
refactored with the following changes:
- Replace call to format with f-string (
use-fstring-for-formatting
)
for variable in variables: | ||
if not variable.description: | ||
errors.append(f'variable {variable.name} has no description') | ||
errors.extend(f'variable {variable.name} has no description' | ||
for variable in variables if not variable.description) | ||
if sorted(variable_names) != variable_names: | ||
message = f'variable order should be: {sorted(variable_names)}' | ||
errors.append(message) | ||
|
||
outputs = tfdoc.get_outputs(subpath) | ||
output_names = [v.name for v in outputs] | ||
for output in outputs: | ||
if not output.description: | ||
errors.append(f'output {output.name} has no description') | ||
errors.extend(f'output {output.name} has no description' | ||
for output in outputs if not output.description) |
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.
Function check_path
refactored with the following changes:
- Replace a for append loop with list extend [×2] (
for-append-to-extend
) - Remove empty elif clause (
remove-pass-elif
)
response = requests.get('https://api.github.com/repos/'+ GITHUB_REPOSITORY +'/issues') | ||
return response | ||
return requests.get(f'https://api.github.com/repos/{GITHUB_REPOSITORY}/issues') |
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.
Function open_issue
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
) - Use f-string instead of string concatenation [×2] (
use-fstring-for-concatenation
)
response = requests.get('https://api.github.com/repos/'+ GITHUB_REPOSITORY +'/issues/'+ str(number) +'/comments') | ||
response = requests.get( | ||
f'https://api.github.com/repos/{GITHUB_REPOSITORY}/issues/{str(number)}/comments' | ||
) |
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.
Function issuecommentcheck
refactored with the following changes:
- Use f-string instead of string concatenation [×4] (
use-fstring-for-concatenation
)
for line in format_variables(variables): | ||
buffer.append(line) | ||
buffer.extend(iter(format_variables(variables))) | ||
buffer.append('\n## Outputs\n') | ||
for line in format_outputs(outputs): | ||
buffer.append(line) | ||
buffer.extend(iter(format_outputs(outputs))) |
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.
Function get_doc
refactored with the following changes:
- Replace a for append loop with list extend [×2] (
for-append-to-extend
) - Simplify generator expression [×2] (
simplify-generator
)
m = re.search('(?sm)%s.*%s' % (MARK_BEGIN, MARK_END), readme) | ||
m = re.search(f'(?sm){MARK_BEGIN}.*{MARK_END}', readme) |
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.
Function replace_doc
refactored with the following changes:
- Replace interpolated string formatting with f-string [×2] (
replace-interpolation-with-fstring
)
variables += [v for v in parse_items( | ||
file.read(), RE_VARIABLES, VariableToken, Variable, VariableData)] | ||
variables += list( | ||
parse_items(file.read(), RE_VARIABLES, VariableToken, Variable, | ||
VariableData)) |
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.
Function get_variables
refactored with the following changes:
- Replace identity comprehension with call to collection constructor (
identity-comprehension
)
outputs += [o for o in parse_items( | ||
file.read(), RE_OUTPUTS, OutputToken, Output, OutputData)] | ||
outputs += list( | ||
parse_items(file.read(), RE_OUTPUTS, OutputToken, Output, | ||
OutputData)) |
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.
Function get_outputs
refactored with the following changes:
- Replace identity comprehension with call to collection constructor (
identity-comprehension
)
m = re.search('(?sm)%s.*%s' % (MARK_BEGIN, MARK_END), readme) | ||
if not m: | ||
if m := re.search(f'(?sm){MARK_BEGIN}.*{MARK_END}', readme): | ||
return get_doc(variables, outputs) in readme | ||
else: | ||
return | ||
return get_doc(variables, outputs) in readme |
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.
Function check_state
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Lift code into else after jump in control flow (
reintroduce-else
) - Swap if/else branches (
swap-if-else-branches
)
Sourcery Code Quality Report✅ Merging this PR will increase code quality in the affected files by 0.04%.
Here are some functions in these files that still need a tune-up:
Legend and ExplanationThe emojis denote the absolute quality of the code:
The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request. Please see our documentation here for details on how these metrics are calculated. We are actively working on this report - lots more documentation and extra metrics to come! Help us improve this quality report! |
Branch
tuning
refactored by Sourcery.If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
See our documentation here.
Run Sourcery locally
Reduce the feedback loop during development by using the Sourcery editor plugin:
Review changes via command line
To manually merge these changes, make sure you're on the
tuning
branch, then run:Help us improve this pull request!