-
Notifications
You must be signed in to change notification settings - Fork 3
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
Csv (Sourcery refactored) #480
Conversation
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.
Due to GitHub API limits, only the first 60 comments can be shown.
self.logger.info("Selecting computer manager: %s" % name) | ||
self.logger.info(f"Selecting computer manager: {name}") |
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 ComputerManager.select
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
self.logger.debug("Registering computer manager %s / %s" % (name, str(klass))) | ||
self.logger.debug(f"Registering computer manager {name} / {str(klass)}") |
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 ComputerManager.register
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
self.logger.error("Selected computer manager '%s' not available" % self.main) | ||
self.logger.error(f"Selected computer manager '{self.main}' not available") |
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 ComputerManager.validate
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
self.logger.debug("getComputer %s" % filt) | ||
self.logger.debug(f"getComputer {filt}") |
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 ComputerManager.getComputer
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
if ret == None: | ||
if ret is 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.
Function ComputerManager.getComputersListHeaders
refactored with the following changes:
- Use x is None rather than x == None (
none-compare
)
if not isDynamicEnable(): | ||
return False | ||
return xmlrpcCleanup(queryManager.getPossiblesCriterionsInModule(ctx, moduleName)) | ||
return ( | ||
xmlrpcCleanup( | ||
queryManager.getPossiblesCriterionsInModule(ctx, moduleName) | ||
) | ||
if isDynamicEnable() | ||
else 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.
Function RpcProxy.getPossiblesCriterionsInModule
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Swap if/else branches of if expression to remove negation (
swap-if-expression
)
if not isDynamicEnable(): | ||
return False | ||
return xmlrpcCleanup(queryManager.getTypeForCriterionInModule(ctx, moduleName, criterion)) | ||
return ( | ||
xmlrpcCleanup( | ||
queryManager.getTypeForCriterionInModule( | ||
ctx, moduleName, criterion | ||
) | ||
) | ||
if isDynamicEnable() | ||
else 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.
Function RpcProxy.getTypeForCriterionInModule
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Swap if/else branches of if expression to remove negation (
swap-if-expression
)
if not isDynamicEnable(): | ||
return False | ||
return xmlrpcCleanup(queryManager.getPossiblesValuesForCriterionInModule(ctx, moduleName, criterion)) | ||
return ( | ||
xmlrpcCleanup( | ||
queryManager.getPossiblesValuesForCriterionInModule( | ||
ctx, moduleName, criterion | ||
) | ||
) | ||
if isDynamicEnable() | ||
else 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.
Function RpcProxy.getPossiblesValuesForCriterionInModule
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Swap if/else branches of if expression to remove negation (
swap-if-expression
)
need_update = {} | ||
for m in machines: | ||
if m['hostname'] != cache[m['uuid']]: | ||
need_update[m['uuid']] = m['hostname'] | ||
|
||
need_update = { | ||
m['uuid']: m['hostname'] | ||
for m in machines | ||
if m['hostname'] != cache[m['uuid']] | ||
} |
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 RpcProxy.update_machine_cache
refactored with the following changes:
- Convert for loop into dictionary comprehension (
dict-comprehension
)
if not isDynamicEnable(): | ||
return "" | ||
else: | ||
return queryManager.getExtended(moduleName, criterion) | ||
return ( | ||
queryManager.getExtended(moduleName, criterion) | ||
if isDynamicEnable() | ||
else "" | ||
) |
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 RpcProxy.getExtended
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
) - Swap if/else branches of if expression to remove negation (
swap-if-expression
)
if query == None: return [] | ||
if __onlyIn(query, ComputerManager().main): | ||
ComputerManager().main | ||
filt = __addCtxFilters(ctx, filt) | ||
filt['query'] = query | ||
return xmlrpcCleanup(ComputerManager().getRestrictedComputersList(ctx, min, max, filt, False, justId, toH)) | ||
else: | ||
if query is None: return [] | ||
if not __onlyIn(query, ComputerManager().main): | ||
return xmlrpcCleanup(QueryManager().replyToQuery(ctx, query, bool, min, max)) | ||
ComputerManager().main | ||
filt = __addCtxFilters(ctx, filt) | ||
filt['query'] = query | ||
return xmlrpcCleanup(ComputerManager().getRestrictedComputersList(ctx, min, max, filt, False, justId, toH)) |
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 replyToQuery
refactored with the following changes:
- Use x is None rather than x == None (
none-compare
) - Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
if query == None: return 0 | ||
if __onlyIn(query, ComputerManager().main): | ||
ComputerManager().main | ||
filt = __addCtxFilters(ctx, filt) | ||
filt['query'] = query | ||
return xmlrpcCleanup(ComputerManager().getRestrictedComputersListLen(ctx, filt)) | ||
else: | ||
if query is None: return 0 | ||
if not __onlyIn(query, ComputerManager().main): | ||
return xmlrpcCleanup(QueryManager().replyToQueryLen(ctx, query, bool)) | ||
ComputerManager().main | ||
filt = __addCtxFilters(ctx, filt) | ||
filt['query'] = query | ||
return xmlrpcCleanup(ComputerManager().getRestrictedComputersListLen(ctx, filt)) |
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 replyToQueryLen
refactored with the following changes:
- Use x is None rather than x == None (
none-compare
) - Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
i = 1 | ||
module = ComputerManager().main | ||
crit = elt | ||
requests = [] | ||
bools = [] | ||
optimization = True | ||
for val in values: | ||
for i, val in enumerate(values, start=1): | ||
# If there is a wildcard in a value, we don't flag this request for | ||
# possible optimization | ||
if optimization: | ||
optimization = not "*" in val | ||
optimization = "*" not in val | ||
requests.append("%i==%s::%s==%s" % (i, module, crit, val)) | ||
bools.append(str(i)) | ||
i += 1 |
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 forgeRequest
refactored with the following changes:
- Move assignment closer to its usage within a block (
move-assign-in-block
) - Simplify logical expression using De Morgan identities (
de-morgan
) - Replace manual loop counter with call to enumerate (
convert-to-enumerate
)
if 'gid' in params: | ||
return self.__getMachines(ctx, params['gid']) | ||
return [] | ||
return self.__getMachines(ctx, params['gid']) if 'gid' in params else [] |
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 DyngroupDatabase.getMachines
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
)
if join_tables == None: | ||
if join_tables is 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.
Function DyngroupDatabase.__getMachinesFirstStep
refactored with the following changes:
- Use x is None rather than x == None (
none-compare
)
result = {} | ||
for machine in query: | ||
result[machine.uuid] = { | ||
"id":machine.id, | ||
"uuid":machine.uuid, | ||
"hostname":machine.name | ||
return { | ||
machine.uuid: { | ||
"id": machine.id, | ||
"uuid": machine.uuid, | ||
"hostname": machine.name, | ||
} | ||
|
||
return result | ||
for machine in query | ||
} |
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 DyngroupDatabase.getRegisteredMachines
refactored with the following changes:
- Convert for loop into dictionary comprehension (
dict-comprehension
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
group = self.__get_group_permissions_request(ctx, session).filter(self.groups.c.id == id).first() | ||
if group: | ||
if ( | ||
group := self.__get_group_permissions_request(ctx, session) | ||
.filter(self.groups.c.id == id) | ||
.first() | ||
): |
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 DyngroupDatabase.setname_group
refactored with the following changes:
- Use named expression to simplify assignment and conditional (
use-named-expression
)
if not s: | ||
return False | ||
return (s.display_in_menu == 1) | ||
return (s.display_in_menu == 1) if s else 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.
Function DyngroupDatabase.canshow_group
refactored with the following changes:
- Lift code into else after jump in control flow (
reintroduce-else
) - Replace if statement with if expression (
assign-if-exp
) - Swap if/else branches of if expression to remove negation (
swap-if-expression
)
return (q != None and q != '') | ||
return q not in [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.
Function DyngroupDatabase.isdyn_group
refactored with the following changes:
- Replace multiple comparisons of same variable with
in
operator (merge-comparisons
)
if g: | ||
return (g.type == 1) | ||
else: | ||
return False | ||
return (g.type == 1) if g else 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.
Function DyngroupDatabase.isprofile
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
if group == None: return [] | ||
if group is None: return [] |
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 DyngroupDatabase.__getContent
refactored with the following changes:
- Use x is None rather than x == None (
none-compare
)
content = self.__getContent(ctx, group, queryManager) | ||
return content | ||
return self.__getContent(ctx, group, queryManager) |
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 DyngroupDatabase.result_group_by_name
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
if justId: | ||
ret = map(lambda m:m.uuid, result.all()) | ||
else: | ||
ret = result.all() | ||
ret = map(lambda m:m.uuid, result.all()) if justId else result.all() |
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 DyngroupDatabase.result_group
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
if self.isprofile(ctx, id): | ||
resultTable = self.profilesResults | ||
else: | ||
resultTable = self.results | ||
|
||
resultTable = self.profilesResults if self.isprofile(ctx, id) else self.results |
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 DyngroupDatabase.delmembers_to_group
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
self.logger.error('Error where fetching deploy group for group %s (package %s): %s' % (gid, package_id, e)) | ||
self.logger.error( | ||
f'Error where fetching deploy group for group {gid} (package {package_id}): {e}' | ||
) |
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 DyngroupDatabase.get_deploy_group_id
refactored with the following changes:
- Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
)
if filt == None or filt == '': | ||
if filt is None or filt == '': |
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 GlpiComputers.getRestrictedComputersListLen
refactored with the following changes:
- Use x is None rather than x == None (
none-compare
)
if filt == None or filt == '': | ||
if filt is None or filt == '': |
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 GlpiComputers.getRestrictedComputersList
refactored with the following changes:
- Use x is None rather than x == None (
none-compare
) - Replace interpolated string formatting with f-string (
replace-interpolation-with-fstring
) - Hoist conditional out of nested conditional [×2] (
hoist-if-from-if
)
result = self.glpi.mini_computers_count() | ||
return result | ||
return self.glpi.mini_computers_count() |
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 GlpiComputers.simple_computer_count
refactored with the following changes:
- Inline variable that is immediately returned (
inline-immediately-returned-variable
)
if filt == None or filt == '': | ||
if filt is None or filt == '': |
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 GlpiComputers.getComputerCount
refactored with the following changes:
- Use x is None rather than x == None (
none-compare
)
if len(ret) != 0: | ||
return ret[0] | ||
else: | ||
return None | ||
return ret[0] if len(ret) != 0 else 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.
Function GlpiComputers.getComputerByMac
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
Pull Request #447 refactored by Sourcery.
If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.
NOTE: As code is pushed to the original Pull Request, Sourcery will
re-run and update (force-push) this Pull Request with new refactorings as
necessary. If Sourcery finds no refactorings at any point, this Pull Request
will be closed automatically.
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
csv
branch, then run:Help us improve this pull request!