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

Restore recognition of deprecated FIDESCTL__API__ ENV variables #965

Merged
merged 8 commits into from
Aug 5, 2022
Merged

Restore recognition of deprecated FIDESCTL__API__ ENV variables #965

merged 8 commits into from
Aug 5, 2022

Conversation

PSalant726
Copy link
Contributor

@PSalant726 PSalant726 commented Aug 4, 2022 •
edited
Loading

Closes #963

Code Changes

  • When generating the config object, search the local ENV for variables prefixed with FIDESCTL__API__, and set any corresponding [database] config values

Steps to Confirm

  • Set an old ENV variable (i.e. FIDESCTL__API__DATABASE_HOST)
  • Start the server
  • Confirm that config.database.server is set to the value of FIDESCTL__API__DATABASE_HOST

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

allisonking and others added 3 commits August 4, 2022 08:52
@allisonking @sanders41
Add workflow for uploading to testpypi (#951)
a8c9653 
* Add workflow for uploading to testpypi

* Update token name

* Change versioning style ti pep440-pre

* Remove debugging trigger on PR

* Update changelog

* Try to properly tag version

* Add a debugging test

* Trigger build

* Edit the right file

* Remove debugging

* Update .github/workflows/publish_package_testpypi.yaml

Co-authored-by: Paul Sanders <[email protected]>

* Combine test pypi logic with original publish package workflow

* Add debugging trigger to workflow

* Fix syntax error

* Install wheel

* Remove debugging

Co-authored-by: Paul Sanders <[email protected]>
@PSalant726
Restore recognition of deprecated FIDESCTL__API__ ENV variables
c044cf9
@PSalant726
Manually rename misaligned options
30e7f4d
@PSalant726 PSalant726 added the bug Something isn't working label Aug 4, 2022
@PSalant726 PSalant726 requested a review from a team August 4, 2022 23:48
@PSalant726 PSalant726 self-assigned this Aug 4, 2022
@PSalant726 PSalant726 marked this pull request as draft August 5, 2022 00:02
NevilleS
NevilleS approved these changes Aug 5, 2022
View reviewed changes
Copy link
Contributor

@NevilleS NevilleS left a comment

Choose a reason for hiding this comment

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

LGTM, but you should get one more review from a core team member who groks the style better 👍

Thanks

tests/ctl/core/test_config.py Show resolved Hide resolved
@PSalant726 PSalant726 marked this pull request as ready for review August 5, 2022 00:19
ThomasLaPiana
ThomasLaPiana approved these changes Aug 5, 2022
View reviewed changes
Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

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

LGTM! great fix

@PSalant726 PSalant726 changed the base branch from main to release-v1.8.1 August 5, 2022 15:57
@PSalant726 PSalant726 merged commit c350bb8 into release-v1.8.1 Aug 5, 2022
@PSalant726 PSalant726 deleted the restore-recognition-of-deprecated-env-vars branch August 5, 2022 15:58
@PSalant726 PSalant726 mentioned this pull request Aug 5, 2022
Closed
ThomasLaPiana added a commit that referenced this pull request Aug 8, 2022
@ThomasLaPiana @PSalant726
@allisonking @sanders41
Merge Release v1.8.1 branch (#971)
c582b83 
* Document the patch release process (#955)

* Update patch release process docs

* Update `CHANGELOG.md`

* Remove `$` character from example commands

* Use mkdocs tip and warning

* nits and a small formatting fix

* Revert "nits and a small formatting fix"

This reverts commit f9b2559.

* Cleaner spacing between sections

* Diction improvements

* Move release notes to the correct version

Co-authored-by: Thomas <[email protected]>

* Remove old env vars (#964)

* Add workflow for uploading to testpypi (#951)

* Add workflow for uploading to testpypi

* Update token name

* Change versioning style ti pep440-pre

* Remove debugging trigger on PR

* Update changelog

* Try to properly tag version

* Add a debugging test

* Trigger build

* Edit the right file

* Remove debugging

* Update .github/workflows/publish_package_testpypi.yaml

Co-authored-by: Paul Sanders <[email protected]>

* Combine test pypi logic with original publish package workflow

* Add debugging trigger to workflow

* Fix syntax error

* Install wheel

* Remove debugging

Co-authored-by: Paul Sanders <[email protected]>

* Update `docker-compose` ENV for new config

* Remove outdated ENV variable references

* Update `CHANGELOG.md`

Co-authored-by: Allison King <[email protected]>
Co-authored-by: Paul Sanders <[email protected]>

* Restore recognition of deprecated `FIDESCTL__API__` ENV variables (#965)

* Add workflow for uploading to testpypi (#951)

* Add workflow for uploading to testpypi

* Update token name

* Change versioning style ti pep440-pre

* Remove debugging trigger on PR

* Update changelog

* Try to properly tag version

* Add a debugging test

* Trigger build

* Edit the right file

* Remove debugging

* Update .github/workflows/publish_package_testpypi.yaml

Co-authored-by: Paul Sanders <[email protected]>

* Combine test pypi logic with original publish package workflow

* Add debugging trigger to workflow

* Fix syntax error

* Install wheel

* Remove debugging

Co-authored-by: Paul Sanders <[email protected]>

* Restore recognition of deprecated `FIDESCTL__API__` ENV variables

* Manually rename misaligned options

* Remove usage of `removeprefix()`

* More robust unit testing

* Update `CHANGELOG.md`

* List deprecated variables and their replacements

Co-authored-by: Allison King <[email protected]>
Co-authored-by: Paul Sanders <[email protected]>

* Fix `fides --version` showing dirty in releases (#962)

* Fix `fides --version` showing dirty in releases

* update the gitignore

* Add workflow for uploading to testpypi (#951)

* Add workflow for uploading to testpypi

* Update token name

* Change versioning style ti pep440-pre

* Remove debugging trigger on PR

* Update changelog

* Try to properly tag version

* Add a debugging test

* Trigger build

* Edit the right file

* Remove debugging

* Update .github/workflows/publish_package_testpypi.yaml

Co-authored-by: Paul Sanders <[email protected]>

* Combine test pypi logic with original publish package workflow

* Add debugging trigger to workflow

* Fix syntax error

* Install wheel

* Remove debugging

Co-authored-by: Paul Sanders <[email protected]>

* remove extra line

* update gitignore

* add a line to the Dockerfile that resets the busted cache

* add the no-deps flag to the fidesctl pip install

* update the git ignore to test the change

* modify publish_package workflow to debug in CI

* revert last commit

* update CI to debug

* debug in CI

* tweak the test paths for CI

* fix the publish_package workflow

* tweak when the publish_package workflow is triggered

* fix the wheel build command

* update the changelog

* Update .github/workflows/publish_package.yaml

Co-authored-by: Allison King <[email protected]>
Co-authored-by: Paul Sanders <[email protected]>

Co-authored-by: Phil Salant <[email protected]>
Co-authored-by: Allison King <[email protected]>
Co-authored-by: Paul Sanders <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ThomasLaPiana ThomasLaPiana ThomasLaPiana approved these changes

@NevilleS NevilleS NevilleS approved these changes

Assignees

@PSalant726 PSalant726

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

Successfully merging this pull request may close these issues.

Env variable not overriding database host config in 1.8.0
4 participants
@PSalant726 @NevilleS @ThomasLaPiana @allisonking