Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Development image to test PopPunk branches #50

Merged
merged 29 commits into from
Dec 5, 2024
Merged

Conversation

absternator
Copy link
Contributor

@absternator absternator commented Nov 27, 2024

This PR creates a new dev image that will can be used to deploy specific versions of poppunk. These images have a -dev postfix. Currently to test new poppunk code, it has to be merged and deployed to bioconda. This can take a while slowing us down.

Testing:
Can test using beebop branch poppunk-version-dev. PR-> pull request #50.
ALSO deployed to dev currently as well

Copy link
Contributor

@EmmaLRussell EmmaLRussell left a comment

Choose a reason for hiding this comment

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

When I run this through beebop, by setting API_BRANCH and running the dependencies script, the worker container runs, but the api container crashes on start with the following logs:

Skipping virtualenv creation, as specified in config file.
/tmp/tmpqhmv2dg2: line 3:    16 Illegal instruction     (core dumped) poetry run waitress-serve --port=5000 beebop.app:app
ERROR conda.cli.main_run:execute(125): `conda run poetry run waitress-serve --port=5000 beebop.app:app` failed. (See above for error)

Also - I'd suggest to only do a dev build when you need to, to minimise build time.

I feel like this could also use an explanatory note in beebop itself, especially if it's going to mean changing API_BRANCH to something which isn't a branch - maybe we should call that API_IMAGE or something..

README.md Outdated
Comment on lines 106 to 109

To use deploy a specific version/commit/branch of PopPUNK, you can update `RUN pip install git+https://github.com/bacpop/PopPUNK@v2.6.7#egg=PopPUNK `
in `DockerFile.dev` with the desired version/commit/branch.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think making a specific reference to v2.6.7 is going to go out of date, so could you rephrase this?

Suggested change
To use deploy a specific version/commit/branch of PopPUNK, you can update `RUN pip install git+https://github.com/bacpop/PopPUNK@v2.6.7#egg=PopPUNK `
in `DockerFile.dev` with the desired version/commit/branch.
To use a specific version, commit or branch of PopPUNK in a beebop_py deployment, you can update the line which installs PopPUNK in `DockerFile.dev to`RUN pip install git+https://github.com/bacpop/PopPUNK@[VERSION]#egg=PopPUNK `, replacing `[VERSION]` with the desired version/commit/branch.

README.md Outdated
To use deploy a specific version/commit/branch of PopPUNK, you can update `RUN pip install git+https://github.com/bacpop/PopPUNK@v2.6.7#egg=PopPUNK `
in `DockerFile.dev` with the desired version/commit/branch.

The new images built with `/docker/build` will have a *-dev* postfix.
Copy link
Contributor

Choose a reason for hiding this comment

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

The build script is always building both prod and dev branches, right? It feels like maybe they should be in separate scripts, or maybe there's a conditional for building the dev image since you won't always need to build it, in every branch.
Maybe there could be a POPPUNK_VERSION variable in common and the build script should build the dev image only if that is set - and in that case it could inject it into the dev image build as an environment variable. Similarly for push.

@absternator
Copy link
Contributor Author

Have updated to only produce dev images as needed with flag --with-dev... also moved to ghcr as there were issues with buildkite and images. (mantra found due to different architecture)

@absternator
Copy link
Contributor Author

TODO: remove buildkite agent after testing all works and deployed on dev!!

Copy link
Contributor

@EmmaLRussell EmmaLRussell left a comment

Choose a reason for hiding this comment

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

Looks good! Worked for me when I ran from the new registry as beebop dep.
Couple of tiny text suggestions.

password: ${{ secrets.GITHUB_TOKEN }}
- name: Build docker image
run: ./docker/build --with-dev # TODO: remove --with_dev before merging
- name: Push SHA docker image
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Push SHA docker image
- name: Push docker image

This pushes both branch and tag doesn't it? So this name is a bit misleading.

In some projects we delay pushing the branch tag until tests have passed - can't remember what we were doing in this one on buildkite.

README.md Outdated
@@ -101,3 +101,19 @@ Testing can be done in a second terminal (make sure to activate 'beebop_py') by
```
TESTING=True poetry run pytest
```

### Use/Deploy specific version of PopPUNK
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
### Use/Deploy specific version of PopPUNK
## Use/Deploy specific version of PopPUNK

I think the next two sections are about this top, so this should be a higher level heading.

@absternator absternator merged commit 0a063d0 into main Dec 5, 2024
4 checks passed
@absternator absternator deleted the poppunk-version-dev branch December 5, 2024 10:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants