You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sebastianliebscher (via GitHub)" <gi...@apache.org> on 2023/08/17 15:39:09 UTC
[GitHub] [superset] sebastianliebscher opened a new pull request, #25016: chore: use os.getenv to simplify superset_config.py
sebastianliebscher opened a new pull request, #25016:
URL: https://github.com/apache/superset/pull/25016
### SUMMARY
This PR removes a function in `superset_config.py` and replaces it by built-in `os.getenv()`. Instead of raising an exception when the env variable is missing, they now have a default value. Their default values are the same as in `.env` file.
### TESTING INSTRUCTIONS
Changes in `superset_config.py` only affect docker deployments, so in order to test this PR run `docker-compose up` and check variables values.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #25016: chore: use os.getenv to simplify superset_config.py
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on PR #25016:
URL: https://github.com/apache/superset/pull/25016#issuecomment-1686637966
@sebastianliebscher I'm a tad confused. You linked to the [.env-non-dev](https://github.com/sebastianliebscher/superset/blob/master/docker/.env-non-dev) file which would indicate that now we have two places (as opposed to one) where the same defaults are defined and thus this doesn't seem to adhere to the DRY principle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] john-bodley commented on pull request #25016: chore: use os.getenv to simplify superset_config.py
Posted by "john-bodley (via GitHub)" <gi...@apache.org>.
john-bodley commented on PR #25016:
URL: https://github.com/apache/superset/pull/25016#issuecomment-1683318718
@sebastianliebscher would you mind describing where the defaults are coming from via either updating your PR description or adding inline review comments?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] sebastianliebscher commented on pull request #25016: chore: use os.getenv to simplify superset_config.py
Posted by "sebastianliebscher (via GitHub)" <gi...@apache.org>.
sebastianliebscher commented on PR #25016:
URL: https://github.com/apache/superset/pull/25016#issuecomment-1697591364
Since I have only now come to it: For completeness @john-bodley's concerns were resolved in https://github.com/apache/superset/pull/25055.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] sebastianliebscher commented on pull request #25016: chore: use os.getenv to simplify superset_config.py
Posted by "sebastianliebscher (via GitHub)" <gi...@apache.org>.
sebastianliebscher commented on PR #25016:
URL: https://github.com/apache/superset/pull/25016#issuecomment-1682749301
Ah, OK. There are new variables from a new commit in master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] sebastianliebscher commented on pull request #25016: chore: use os.getenv to simplify superset_config.py
Posted by "sebastianliebscher (via GitHub)" <gi...@apache.org>.
sebastianliebscher commented on PR #25016:
URL: https://github.com/apache/superset/pull/25016#issuecomment-1682737811
`Python Misc / pre-commit` is failing with
```bash
mypy.....................................................................Failed
- hook id: mypy
- exit code: 1
docker/pythonpath_dev/superset_config.py:38: error: Name "get_env_variable" is not defined [name-defined]
docker/pythonpath_dev/superset_config.py:39: error: Name "get_env_variable" is not defined [name-defined]
docker/pythonpath_dev/superset_config.py:40: error: Name "get_env_variable" is not defined [name-defined]
docker/pythonpath_dev/superset_config.py:41: error: Name "get_env_variable" is not defined [name-defined]
docker/pythonpath_dev/superset_config.py:42: error: Name "get_env_variable" is not defined [name-defined]
```
This is the function I removed with this PR. Locally, all pre-commit checks run successfully.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina merged pull request #25016: chore: use os.getenv to simplify superset_config.py
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina merged PR #25016:
URL: https://github.com/apache/superset/pull/25016
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org