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

adding gunicorn instead of uwsgi and modifying the base image. #362

Open
wants to merge 44 commits into
base: master
Choose a base branch
from

Conversation

EliseCastle23
Copy link
Contributor

I am removing uwsgi to be replaced by gunicorn and making modifications to use the new python3.9 base image. I had to make some modifications for indexd to be compatible with the AL2 base image.

@github-actions
Copy link

The style in this PR agrees with black. ✔️

This formatting comment was generated automatically by a script in uc-cdis/wool.

@EliseCastle23 EliseCastle23 marked this pull request as ready for review September 22, 2023 16:16
Copy link

github-actions bot commented Sep 4, 2024

Please find the ci env pod logs here

Copy link

github-actions bot commented Sep 4, 2024

Please find the ci env pod logs here

Copy link

github-actions bot commented Sep 5, 2024

filepath $$\textcolor{#23d18b}{\tt{passed}}$$ $$\textcolor{#f14c4c}{\tt{failed}}$$ $$\textcolor{#ffa500}{\tt{skipped}}$$ SUBTOTAL
$$\textcolor{#f14c4c}{\tt{tests/test\_centralized\_auth.py}}$$ $$\textcolor{#23d18b}{\tt{9}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{16}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_data\_upload.py}}$$ $$\textcolor{#23d18b}{\tt{6}}$$ $$\textcolor{#f14c4c}{\tt{2}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{9}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_graph\_submit\_and\_query.py}}$$ $$\textcolor{#23d18b}{\tt{6}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_presigned\_url.py}}$$ $$\textcolor{#23d18b}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{7}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_dbgap.py}}$$ $$\textcolor{#23d18b}{\tt{3}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#ffa500}{\tt{4}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_audit\_service.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_gen3client.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#f14c4c}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{33}}$$ $$\textcolor{#f14c4c}{\tt{10}}$$ $$\textcolor{#ffa500}{\tt{2}}$$ $$\textcolor{#f14c4c}{\tt{45}}$$

Please find the detailed integration test report here

Please find the ci env pod logs here

Copy link

github-actions bot commented Sep 6, 2024

filepath $$\textcolor{#23d18b}{\tt{passed}}$$ $$\textcolor{#f14c4c}{\tt{failed}}$$ $$\textcolor{#f14c4c}{\tt{error}}$$ $$\textcolor{#ffa500}{\tt{skipped}}$$ SUBTOTAL
$$\textcolor{#f14c4c}{\tt{tests/test\_data\_upload.py}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{9}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{9}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_graph\_submit\_and\_query.py}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_centralized\_auth.py}}$$ $$\textcolor{#23d18b}{\tt{9}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{16}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_audit\_service.py}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_presigned\_url.py}}$$ $$\textcolor{#23d18b}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{7}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_dbgap.py}}$$ $$\textcolor{#23d18b}{\tt{3}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#ffa500}{\tt{4}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_gen3client.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#f14c4c}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{20}}$$ $$\textcolor{#f14c4c}{\tt{8}}$$ $$\textcolor{#f14c4c}{\tt{16}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{45}}$$

Please find the detailed integration test report here

Please find the ci env pod logs here

Copy link

filepath $$\textcolor{#23d18b}{\tt{passed}}$$ $$\textcolor{#ffa500}{\tt{skipped}}$$ SUBTOTAL
$$\textcolor{#23d18b}{\tt{tests/test\_centralized\_auth.py}}$$ $$\textcolor{#23d18b}{\tt{16}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{16}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_data\_upload.py}}$$ $$\textcolor{#23d18b}{\tt{8}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#ffa500}{\tt{9}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_graph\_submit\_and\_query.py}}$$ $$\textcolor{#23d18b}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{7}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_dbgap.py}}$$ $$\textcolor{#23d18b}{\tt{3}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#ffa500}{\tt{4}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_presigned\_url.py}}$$ $$\textcolor{#23d18b}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{7}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_audit\_service.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_gen3client.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#ffa500}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{43}}$$ $$\textcolor{#ffa500}{\tt{2}}$$ $$\textcolor{#ffa500}{\tt{45}}$$

Please find the detailed integration test report here

Please find the ci env pod logs here

Copy link

filepath $$\textcolor{#23d18b}{\tt{passed}}$$ $$\textcolor{#ffa500}{\tt{skipped}}$$ SUBTOTAL
$$\textcolor{#23d18b}{\tt{tests/test\_centralized\_auth.py}}$$ $$\textcolor{#23d18b}{\tt{16}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{16}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_data\_upload.py}}$$ $$\textcolor{#23d18b}{\tt{8}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#ffa500}{\tt{9}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_graph\_submit\_and\_query.py}}$$ $$\textcolor{#23d18b}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{7}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_presigned\_url.py}}$$ $$\textcolor{#23d18b}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{7}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_dbgap.py}}$$ $$\textcolor{#23d18b}{\tt{3}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#ffa500}{\tt{4}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_audit\_service.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_gen3client.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#ffa500}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{43}}$$ $$\textcolor{#ffa500}{\tt{2}}$$ $$\textcolor{#ffa500}{\tt{45}}$$

Please find the detailed integration test report here

Please find the ci env pod logs here

Dockerfile Outdated
FROM quay.io/cdis/python-build-base:${AZLINUX_BASE_VERSION} AS base

# Comment this in, and comment out the line above, if quay is down
# FROM 707767160287.dkr.ecr.us-east-1.amazonaws.com/gen3/python-build-base:${AZLINUX_BASE_VERSION} as base
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason we're not preferring ECR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So that community may be able to build/ develop and I belive ECR has a bandwidth cost, while quay is free.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh, that makes sense 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that acceptable for our own internal usage? Or do we need some way to automate flipping that to ECR as part of build?

Dockerfile Outdated
USER gen3


RUN python -m venv /venv
Copy link
Contributor

@Avantol13 Avantol13 Oct 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so poetry install will create a venv automatically when you install. And in fact, we're explicitly setting https://python-poetry.org/docs/configuration/#virtualenvsin-project, so it should be deterministic where the venv gets created. So as long as you run all future commands you want within the env with poetry run .... then it should just work.

I don't think we need (or want) any other venv setup or activation outside of the stock poetry support. It could create some confusing differences

Dockerfile Outdated
## For local development
# FROM quay.io/cdis/python-nginx-al2:feat_python-nginx AS base

FROM 707767160287.dkr.ecr.us-east-1.amazonaws.com/gen3/python-nginx-al2:feat_python-nginx as base
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's follow the warning and fix The 'as' keyword should match the case of the 'from' keyword

@@ -0,0 +1,36 @@
version: '3.8'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per previous conversations, I'd like to remove these separate ideas around local dev improvements to speed up the approval and review of the base image upgrades. We can have a separate PR and discussions to talk through ideas like this

pyproject.toml Outdated
@@ -21,13 +21,13 @@ gen3authz = "^1.0.4"
hsclient = {git = "https://github.com/uc-cdis/hsclient", rev = "1.0.0"}
indexclient = "^2.1.0"
jsonschema = "^3.2"
psycopg2-binary = "^2.7"
gunicorn = "^22.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are not blocked from upgrading beyond these versions, I'd like to get in the habit of specifying >= rather than the default ^. If things break, we'll see it in unit tests and either update so the newer lib works, or at that point we'll restrict the versioning and create a ticket to upgrade.

As it is, we'll never automatically try to update to new major versions.

So, even though the other deps aren't like this yet, we could replace all ^ with >= at least with the new deps.

If you're feeling adventurous, we could try everything else too. With the exception of sqlalchemy, which I know won't work b/c 2.0 requires updates

Copy link

filepath $$\textcolor{#23d18b}{\tt{passed}}$$ $$\textcolor{#f14c4c}{\tt{failed}}$$ $$\textcolor{#ffa500}{\tt{skipped}}$$ SUBTOTAL
$$\textcolor{#f14c4c}{\tt{tests/test\_data\_upload.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{9}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_graph\_submit\_and\_query.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{6}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_audit\_service.py}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_gen3client.py}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_centralized\_auth.py}}$$ $$\textcolor{#23d18b}{\tt{16}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{16}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_presigned\_url.py}}$$ $$\textcolor{#23d18b}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{7}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_dbgap.py}}$$ $$\textcolor{#23d18b}{\tt{3}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#ffa500}{\tt{4}}$$
$$\textcolor{#f14c4c}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{28}}$$ $$\textcolor{#f14c4c}{\tt{15}}$$ $$\textcolor{#ffa500}{\tt{2}}$$ $$\textcolor{#f14c4c}{\tt{45}}$$

Please find the detailed integration test report here

Please find the ci env pod logs here

Copy link

Please find the ci env pod logs here

Copy link

filepath $$\textcolor{#23d18b}{\tt{passed}}$$ $$\textcolor{#f14c4c}{\tt{failed}}$$ $$\textcolor{#ffa500}{\tt{skipped}}$$ SUBTOTAL
$$\textcolor{#f14c4c}{\tt{tests/test\_data\_upload.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{9}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_graph\_submit\_and\_query.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{6}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_audit\_service.py}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_gen3client.py}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_centralized\_auth.py}}$$ $$\textcolor{#23d18b}{\tt{16}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{16}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_presigned\_url.py}}$$ $$\textcolor{#23d18b}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{7}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_dbgap.py}}$$ $$\textcolor{#23d18b}{\tt{3}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#ffa500}{\tt{4}}$$
$$\textcolor{#f14c4c}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{28}}$$ $$\textcolor{#f14c4c}{\tt{15}}$$ $$\textcolor{#ffa500}{\tt{2}}$$ $$\textcolor{#f14c4c}{\tt{45}}$$

Please find the detailed integration test report here

Please find the ci env pod logs here

Copy link

filepath $$\textcolor{#23d18b}{\tt{passed}}$$ $$\textcolor{#f14c4c}{\tt{failed}}$$ $$\textcolor{#ffa500}{\tt{skipped}}$$ SUBTOTAL
$$\textcolor{#f14c4c}{\tt{tests/test\_data\_upload.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{9}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_graph\_submit\_and\_query.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#f14c4c}{\tt{6}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{7}}$$
$$\textcolor{#f14c4c}{\tt{tests/test\_gen3client.py}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#f14c4c}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_centralized\_auth.py}}$$ $$\textcolor{#23d18b}{\tt{16}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{16}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_presigned\_url.py}}$$ $$\textcolor{#23d18b}{\tt{7}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{7}}$$
$$\textcolor{#ffa500}{\tt{tests/test\_dbgap.py}}$$ $$\textcolor{#23d18b}{\tt{3}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#ffa500}{\tt{1}}$$ $$\textcolor{#ffa500}{\tt{4}}$$
$$\textcolor{#23d18b}{\tt{tests/test\_audit\_service.py}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#666666}{\tt{0}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#f14c4c}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{29}}$$ $$\textcolor{#f14c4c}{\tt{14}}$$ $$\textcolor{#ffa500}{\tt{2}}$$ $$\textcolor{#f14c4c}{\tt{45}}$$

Please find the detailed integration test report here

Please find the ci env pod logs here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants