-
Notifications
You must be signed in to change notification settings - Fork 3
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
chore: runtime config #310
Conversation
Codecov Report
@@ Coverage Diff @@
## main #310 +/- ##
=======================================
Coverage 76.12% 76.13%
=======================================
Files 90 90
Lines 6660 6662 +2
=======================================
+ Hits 5070 5072 +2
Misses 1590 1590
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
server/eb/app.py
Outdated
@@ -149,6 +149,7 @@ def get_csp_hashes(app, app_config): | |||
if config_location.exists(): | |||
with config_location.local_handle() as lh: | |||
logging.info(f"Configuration from {config_file}") | |||
print(lh) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious -- why print this versus logging it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, this one has to go, thanks for the catch
server/eb/app.py
Outdated
dataroot = os.getenv("CXG_DATAROOT") | ||
if dataroot: | ||
logging.info("Configuration from CXG_DATAROOT") | ||
app_config.update_server_config(multi_dataset__dataroot=dataroot) | ||
|
||
# overwrite configuration for the eb app | ||
# TODO: this doesn't make sense |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we only support cxg matrices in the explorer at this point?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will leave it just in case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
* chore: runtime config (#310) * chore: runtime config * reqs * add config.yaml to the dockerfile * Remove print * Remove comments * Missing ADD argument * fix some tf issues * chore: add missing config.yaml file (#312) * chore: increase staging container count to 2 (#313) * fix: enable deployment on rdev (#299) * Add symlink * Update env config * Update health check path * Add temporary workarounds and warnings * Workaround missing s3 bucket * add request logging * Repair explorer rdev * Remove unused config file * Cleanup * Clean up * Lint * Reorder dockerfile to take advantage of caching * Remove unnecessary installations * Ignore node_modules in docker image to save space * Fix cache-from tag in Docker * fix conflicts * Reorder Dockerfile for caching Co-authored-by: Trent Smith <[email protected]> Co-authored-by: Emanuele Bezzi <[email protected]> * Combine run statements into one layer (#314) * chore: add happy locking (#318) Co-authored-by: Emanuele Bezzi <[email protected]> Co-authored-by: Rohan Agarwal <[email protected]> Co-authored-by: Trent Smith <[email protected]>
Reviewers
Functional:
@roaga @MDunitz @atolopko-czi
Readability:
Changes
config.yaml
file using a library.