-
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
Sourcery Starbot ⭐ refactored danyi1212/django-windowsauth #16
base: main
Are you sure you want to change the base?
Conversation
if domain: | ||
# for specific domain | ||
return user.ldap.domain == domain | ||
else: | ||
# for all domains | ||
return True | ||
return user.ldap.domain == domain if domain else True |
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 domain_required.check_domain
refactored with the following changes:
- Replace if statement with if expression (
assign-if-exp
)
This removes the following comments ( why? ):
# for all domains
# for specific domain
if user.is_authenticated and LDAPUser.objects.filter(user=user).exists(): | ||
try: | ||
if WAUTH_USE_CACHE: | ||
user.ldap.sync() | ||
else: | ||
# check via database query | ||
if not timedelta or not user.ldap.last_sync or user.ldap.last_sync < timezone.now() - timedelta: | ||
user.ldap.sync() | ||
|
||
return True | ||
except Exception as e: | ||
if raise_exception: | ||
raise e | ||
else: | ||
return False | ||
else: | ||
if ( | ||
not user.is_authenticated | ||
or not LDAPUser.objects.filter(user=user).exists() | ||
): | ||
return allow_non_ldap | ||
try: | ||
if WAUTH_USE_CACHE: | ||
user.ldap.sync() | ||
elif not timedelta or not user.ldap.last_sync or user.ldap.last_sync < timezone.now() - timedelta: | ||
user.ldap.sync() | ||
|
||
return True | ||
except Exception as e: | ||
if raise_exception: | ||
raise e | ||
else: | ||
return 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 ldap_sync_required.check_sync
refactored with the following changes:
- Add guard clause (
last-if-guard
) - Merge else clause's nested if statement into elif (
merge-else-if-into-elif
)
This removes the following comments ( why? ):
# check via database query
self.settings = settings if settings else LDAPSettings.for_domain(domain) | ||
self.settings = settings or LDAPSettings.for_domain(domain) |
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 LDAPManager.__init__
refactored with the following changes:
- Simplify if expression by using or (
or-if-exp-identity
)
else: | ||
# check via database query | ||
if not ldap_user.last_sync or ldap_user.last_sync < timezone.now() - timezone.timedelta(seconds=timeout): | ||
ldap_user.sync() | ||
elif not ldap_user.last_sync or ldap_user.last_sync < timezone.now() - timezone.timedelta(seconds=timeout): | ||
ldap_user.sync() | ||
except LDAPUser.DoesNotExist: | ||
# user is getting created the first time | ||
pass | ||
except Exception as e: | ||
logger.exception(f"Failed to synchronize user {request.user} against LDAP") | ||
# return error response | ||
if WAUTH_REQUIRE_RESYNC: | ||
if isinstance(WAUTH_ERROR_RESPONSE, int): | ||
return HttpResponse(f"Authorization Failed.", status=WAUTH_ERROR_RESPONSE) | ||
elif callable(WAUTH_ERROR_RESPONSE): | ||
if isinstance(WAUTH_ERROR_RESPONSE, int): | ||
if WAUTH_REQUIRE_RESYNC: | ||
return HttpResponse("Authorization Failed.", status=WAUTH_ERROR_RESPONSE) | ||
elif callable(WAUTH_ERROR_RESPONSE): | ||
if WAUTH_REQUIRE_RESYNC: | ||
return WAUTH_ERROR_RESPONSE(request, e) | ||
else: | ||
raise e | ||
response = self.get_response(request) | ||
return response | ||
elif WAUTH_REQUIRE_RESYNC: | ||
raise e | ||
return self.get_response(request) |
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 UserSyncMiddleware.__call__
refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif
) - Swap positions of nested conditionals (
swap-nested-ifs
) - Replace f-string with no interpolated values with string (
remove-redundant-fstring
) - Inline variable that is immediately returned (
inline-immediately-returned-variable
)
This removes the following comments ( why? ):
# return error response
# check via database query
elif "\\" in username: | ||
domain, sam_account_name = username.split("\\", 2) | ||
|
||
else: | ||
raise ValueError("Username must be in DOMAIN\\username format.") |
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 LDAPUserManager.create_user
refactored with the following changes:
- Merge else clause's nested if statement into elif (
merge-else-if-into-elif
) - Lift code into else after jump in control flow (
reintroduce-else
) - Swap if/else branches (
swap-if-else-branches
)
return format_bytes(obj.bytes_received) + " / " + format_bytes(obj.bytes_transmitted) | ||
return f'{format_bytes(obj.bytes_received)} / ' + format_bytes( | ||
obj.bytes_transmitted | ||
) |
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 LDAPUsageAdmin.bytes
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
return str(obj.messages_received) + " / " + str(obj.messages_transmitted) | ||
return f'{str(obj.messages_received)} / {str(obj.messages_transmitted)}' |
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 LDAPUsageAdmin.messages
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
return str(obj.open_sockets) + " / " + str(obj.closed_sockets) + " / " + str(obj.wrapped_sockets) | ||
return f'{str(obj.open_sockets)} / {str(obj.closed_sockets)} / ' + str( | ||
obj.wrapped_sockets | ||
) |
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 LDAPUsageAdmin.sockets
refactored with the following changes:
- Use f-string instead of string concatenation (
use-fstring-for-concatenation
)
logger.info(f"Unbinding all LDAP Connections for collecting metrics") | ||
logger.info("Unbinding all LDAP Connections for collecting metrics") |
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 collect_metrics
refactored with the following changes:
- Replace f-string with no interpolated values with string (
remove-redundant-fstring
)
# predefined tasks | ||
if command in PREDEFINED_TASKS: | ||
create_task = PREDEFINED_TASKS[command] | ||
create_task(command=command, name=name, desc=desc, identity=identity, folder=folder, | ||
interval=interval, random=random, timeout=timeout, priority=priority) | ||
else: | ||
if command not in PREDEFINED_TASKS: | ||
raise CommandError(f"Predefined task for \"{command}\" does not exist.") | ||
create_task = PREDEFINED_TASKS[command] | ||
create_task(command=command, name=name, desc=desc, identity=identity, folder=folder, | ||
interval=interval, random=random, timeout=timeout, priority=priority) |
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 Command.handle
refactored with the following changes:
- Swap if/else branches (
swap-if-else-branches
) - Remove unnecessary else after guard condition (
remove-unnecessary-else
)
This removes the following comments ( why? ):
# predefined tasks
Thanks for starring sourcery-ai/sourcery ✨ 🌟 ✨
Here's your pull request refactoring your most popular Python repo.
If you want Sourcery to refactor all your Python repos and incoming pull requests install our bot.
Review changes via command line
To manually merge these changes, make sure you're on the
main
branch, then run: