-
Notifications
You must be signed in to change notification settings - Fork 17
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
Feat/al2 updates #421
base: master
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to move off the branch
Pull Request Test Coverage Report for Build 13081070211Details
💛 - 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 |
…ng to install via yum slightly differently
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 |
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 a couple of minor questions. Will approve once answered.
# The user should be "sheepdog", but for legacy reasons, we use "gdcapi" instead | ||
"auth": ( | ||
( | ||
environ.get("INDEXD_USER", "gdcapi"), |
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 see os.environ.get
and environ.get
both being used. Wanted to make sure this is intentional and not an oversight.
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.
Yes that was intentional and I believe a step Alex mentioned that we needed
pyproject.toml
Outdated
@@ -1,22 +1,25 @@ | |||
[tool.poetry] | |||
name = "sheepdog" | |||
version = "5.1.6" | |||
version = "5.1.5" |
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.
Why did we roll down a version to 5.1.5
?
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'll be honest, I am not entirely sure anymore. Let me roll it forward to 5.1.6 and observe the build
Please find the ci env pod logs here |
Improvements
adding gunicorn and nginx to sheepdog and basing off our new secure python image