Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Change auth token assertion logging level from error to warning #181

Merged
merged 1 commit into from
Oct 12, 2021
Merged

Change auth token assertion logging level from error to warning #181

merged 1 commit into from
Oct 12, 2021

Conversation

leikoilja
Copy link
Owner

Closes #180

@leikoilja leikoilja added the bug Something isn't working label Oct 11, 2021
@leikoilja leikoilja self-assigned this Oct 11, 2021
@leikoilja leikoilja mentioned this pull request Oct 11, 2021
Closed
@leikoilja leikoilja requested review from ArnyminerZ and KapJI October 12, 2021 06:44
@KapJI
Copy link
Collaborator

KapJI commented Oct 12, 2021

I think these checks should be moved out of constructor. Basically we shouldn't create partially initialized objects where not all fields are set.

@leikoilja
Copy link
Owner Author

Agree with you, @KapJI. I think there checks should be moved into classmethod/staticmethod and validation ran before the object is initialised. But we should do it in a separate refactoring PR :)

KapJI
KapJI approved these changes Oct 12, 2021
View reviewed changes
Copy link
Collaborator

@KapJI KapJI left a comment

Choose a reason for hiding this comment

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

This one is good to go then.

@KapJI KapJI merged commit 8761b11 into master Oct 12, 2021
@KapJI KapJI deleted the change-logging-level-#180 branch October 12, 2021 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KapJI KapJI KapJI approved these changes

@ArnyminerZ ArnyminerZ Awaiting requested review from ArnyminerZ

Assignees

@leikoilja leikoilja

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Do not log error for device auth token assertion
2 participants
@leikoilja @KapJI