Skip to content

feat: Prevent overriding the value from config #7218

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

Conversation

irvifa
Copy link
Contributor

@irvifa irvifa commented Dec 9, 2021

Description

Fixes #Previously the DEBUG env var will always be overridden into True, while
we already have a way to read it from env var. By setting the default
value as True and not overridding the value later we can retainthe DEBUG
value provided from the users.

Note: It's a good idea to open an issue first for discussion.

Checklist

@irvifa irvifa requested review from glasnt and a team as code owners December 9, 2021 09:36
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Dec 9, 2021
@irvifa irvifa changed the title feat: Prevent oevrriding the value from config feat: Prevent overriding the value from config Dec 9, 2021
Previously the DEBUG env var will always be overridden into True, while
we already have a way to read it from env var. By setting the default
value as True and not overridding the value later we can retainthe DEBUG
value provided from the users.
@irvifa irvifa force-pushed the feat/enable-reading-debug-from-config branch from 0bc7bed to 3feb3c6 Compare December 9, 2021 09:38
Copy link
Collaborator

@dandhlee dandhlee left a comment

Choose a reason for hiding this comment

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

LGTM, one minor comment.

Comment on lines 64 to 65
# SECURITY WARNING: don't run with debug turned on in production!
# Change this to "False" when you are ready for production
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you move this warning up above to where it's set to True as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed your comment @dandhlee

@dandhlee dandhlee added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 10, 2021
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 10, 2021
@irvifa irvifa requested a review from dandhlee December 25, 2021 07:59
Copy link
Collaborator

@dandhlee dandhlee left a comment

Choose a reason for hiding this comment

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

LGTM.

@dandhlee dandhlee added the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 25, 2021
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 25, 2021
@dandhlee dandhlee added automerge Merge the pull request once unit tests and other checks pass. kokoro:run Add this label to force Kokoro to re-run the tests. labels Dec 25, 2021
@kokoro-team kokoro-team removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Dec 25, 2021
@gcf-merge-on-green gcf-merge-on-green bot merged commit db182f7 into GoogleCloudPlatform:main Dec 25, 2021
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Dec 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
samples Issues that are directly related to samples.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants