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

deprecate warning for not catch lowercase env var #2344

Merged
merged 10 commits into from Dec 26, 2021
Merged

deprecate warning for not catch lowercase env var #2344

merged 10 commits into from Dec 26, 2021

Conversation

ChihweiLHBird
Copy link
Member

@ChihweiLHBird ChihweiLHBird commented Dec 24, 2021 •
edited

Related issue: #2120

@ChihweiLHBird ChihweiLHBird marked this pull request as ready for review December 24, 2021 20:43
@ChihweiLHBird ChihweiLHBird requested a review from a team as a code owner December 24, 2021 20:43
prryplatypus
prryplatypus reviewed Dec 25, 2021
View reviewed changes
sanic/config.py Outdated Show resolved Hide resolved
prryplatypus
prryplatypus previously approved these changes Dec 25, 2021
View reviewed changes
ahopkins
ahopkins requested changes Dec 25, 2021
View reviewed changes
Copy link
Member

@ahopkins ahopkins left a comment

Choose a reason for hiding this comment

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

We should probably only raise this once per load, not for each variable. Also, can you add a unit test for coverage?

sanic/config.py Outdated Show resolved Hide resolved
@ChihweiLHBird @ahopkins
Update sanic/config.py
7dddc7e 
Co-authored-by: Adam Hopkins <adam@amhopkins.com>
@ChihweiLHBird
Copy link
Member Author

@ahopkins Do you think the code climate Cognitive Complexity limit can be increased? I have no idea about how to make it below 10...

@ChihweiLHBird
Copy link
Member Author

Maybe we can wrap this into a function?

for converter in reversed(self._converters):
    try:
        self[config_key] = converter(value)
        break
    except ValueError:
        pass

ahopkins
ahopkins reviewed Dec 26, 2021
View reviewed changes
tests/test_config.py Outdated Show resolved Hide resolved
@ahopkins
Copy link
Member

@ahopkins Do you think the code climate Cognitive Complexity limit can be increased? I have no idea about how to make it below 10...

Don't worry about it. We are going to give up on code climate soon and try to go back to codecov.

@ahopkins ahopkins merged commit 371985d into sanic-org:main Dec 26, 2021
@ChihweiLHBird ChihweiLHBird deleted the zhiwei/deprecation-lowercase-config branch December 26, 2021 20:19
@ChihweiLHBird ChihweiLHBird changed the title deprecation warning for not catch lowercase env var deprecate warning for not catch lowercase env var Dec 28, 2021
@ahopkins ahopkins mentioned this pull request Mar 30, 2022
Merged
ChihweiLHBird added a commit to ChihweiLHBird/sanic that referenced this pull request Jun 1, 2022
@ChihweiLHBird
deprecation warning for not catch lowercase env var (sanic-org#2344)
2026bd7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prryplatypus prryplatypus prryplatypus left review comments

@ahopkins ahopkins ahopkins approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ChihweiLHBird @ahopkins @prryplatypus