Skip to content

Conversation

vlebourl
Copy link
Contributor

@vlebourl vlebourl commented Apr 8, 2022

Not sure you want to appy that, so I'll make it a draft for now. But I think it's relevant!

@vlebourl vlebourl marked this pull request as draft April 8, 2022 08:21
@github-actions github-actions bot added the enhancement New feature or request label Apr 8, 2022
TooManyConcurrentRequestsException,
TooManyExecutionsException,
TooManyRequestsException,
UnknownException,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery - Raise specific exception instead of genericException

self._states = [State(**state) for state in states]
else:
self._states = []
self._states = [State(**state) for state in states] if states else []
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery - Replace if statement with if expression

self.event_listener_id: str | None = None

self.session = session if session else ClientSession()
self.session = session or ClientSession()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery - Simplify if expression by using or

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed

# Request access token
async with self.session.post(
SOMFY_API + "/oauth/oauth/v2/token",
f"{SOMFY_API}/oauth/oauth/v2/token",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery - Use f-string instead of string concatenation

# Request access token
async with self.session.post(
SOMFY_API + "/oauth/oauth/v2/token",
f"{SOMFY_API}/oauth/oauth/v2/token",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery - Use f-string instead of string concatenation

if "Server is down for maintenance" in result:
raise MaintenanceException("Server is down for maintenance") from error
raise Exception(
raise UnknownException(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery - Raise a specific error instead of the general Exception.

raise NotSuchTokenException(message)

raise Exception(message if message else result)
raise UnknownException(message or result)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery - Raise a specific error instead of the general Exception.
Sourcery - Simplify if expression by using or

pass


class UnknownException(Exception):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery - Raise a specific error instead of the general Exception.

def obfuscate_string(input: str) -> str:
"""Mask string"""
return re.sub(r"[a-zA-Z0-9_.-]*", "*", str(input))
return re.sub(r"[a-zA-Z0-9_.-]*", "*", input)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sourcery - Remove unnecessary casts to int, str, float or bool

jwt = jwt.strip('"') # Remove surrounding quotes

return jwt
return str(jwt)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mypy: pyoverkiz/client.py:302: error: Returning Any from function declared to return "str" [no-any-return]

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should cast it if we know it is a string for sure.

Copy link
Owner

@iMicknl iMicknl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Let's see if we can implement a few of these changes / fixes :).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants