Feat/al2 updates #421
Feat/al2 updates #421
Conversation
|
The style in this PR agrees with This formatting comment was generated automatically by a script in uc-cdis/wool. |
|
Please find the ci env pod logs here |
|
Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
deployment/wsgi/gunicorn.conf.py
Outdated
|
|
||
|
|
||
| def pre_fork(server, worker): | ||
| time.sleep(random.uniform(0, 2)) |
There was a problem hiding this comment.
can we add some comments about the reasoning for this? And is there a reason we do this here but not in other services like indexd?
pyproject.toml
Outdated
| requests = ">=2.31.0,<3.0.0" | ||
| simplejson = ">=3.8.1" | ||
| sqlalchemy = ">=1.3.*" | ||
| cdispyutils = ">=2.0.1" | ||
| psqlgraph = ">=3.0.1" | ||
| cdiserrors = ">=1.0.0" | ||
| cdislogging = ">=1.0.0" | ||
| gen3dictionary = ">=2.0.3" | ||
| gen3datamodel = ">=3.1.0" | ||
| gen3dictionary = {git = "https://github.com/uc-cdis/datadictionary.git", branch = "chore/add-unit-tests"} |
There was a problem hiding this comment.
this needs to move off the branch
pyproject.toml
Outdated
| psycopg2 = ">=2.8.4" | ||
| PyYAML = ">=5.4.1" | ||
| psycopg2-binary = ">=2.8.4" | ||
| PyYAML = "*" |
There was a problem hiding this comment.
I'd like to keep using >= so we don't accidentally downgrade
Pull Request Test Coverage Report for Build 11800882673Details
💛 - Coveralls |
|
Please find the ci env pod logs here |
|
Please find the ci env pod logs here |
|
Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
|
Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
Please find the detailed integration test report here Please find the ci env pod logs here |
| @@ -152,10 +152,6 @@ def app_init(app): | |||
| db_init(app) | |||
| # exclude es init as it's not used yet | |||
| # es_init(app) | |||
| try: | |||
| app.secret_key = app.config["FLASK_SECRET_KEY"] | |||
There was a problem hiding this comment.
why are we removing this? Flask requires this to read/write session iirc.
There was a problem hiding this comment.
We were removing settings from GDC, that we could not find references to in code.
Interesting that none of the unit/ integration tests cover this if this is managing sessions 🤔
Happy to put this back in if you think if we still need it.
There was a problem hiding this comment.
It's used internally by Flask. We should not remove it. https://flask.palletsprojects.com/en/stable/quickstart/#sessions
It's possible that sheepdog doesn't use any Flask session stuff right now, but I would rather just leave this in so it's already configured if someone uses it in the future. And if there is use of this, if we remove it, the sessions will all be unreadable
Improvements
adding gunicorn and nginx to sheepdog and basing off our new secure python image