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.

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

Use SUPERSET_CONFIG_PATH env var to manage config #31

Merged
merged 1 commit into from
Jan 29, 2024
Merged

Conversation

kaapstorm
Copy link
Contributor

This change uses the SUPERSET_CONFIG_PATH environment variable to manage Superset configuration, both for tests and for normal operation.

Calling hq_superset.patch_superset_config(superset_config) is unnecessary. The same can be achieved by including

FLASK_APP_MUTATOR = flask_app_mutator
CUSTOM_SECURITY_MANAGER = oauth.CommCareSecurityManager

in the config file.

I've added a comment to the start of superset_config.example.py to explain how to set the environment variable for normal operation. The environment variable is set automatically in hq_superset/tests/base_test.py for running tests.

@kaapstorm kaapstorm requested a review from SmittieC January 24, 2024 15:30
@SmittieC
Copy link

@kaapstorm When I upgrade typing extensions locally, it works, but the version is larger than that which superset wants, so maybe we should merge this branch and see what it does with the later supset version?

@kaapstorm
Copy link
Contributor Author

When I upgrade typing extensions ... see what it does with the later supset version?

I've rebased off master, which should resolve this. I'm watching the tests.

@kaapstorm
Copy link
Contributor Author

Following up failures.

@kaapstorm
Copy link
Contributor Author

Failures were caused by config_for_tests.py diverging from GitHub workflow config.

@kaapstorm kaapstorm merged commit 2e061f7 into master Jan 29, 2024
3 checks passed
@kaapstorm kaapstorm deleted the nh/config branch January 29, 2024 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants