Skip to content

test: run the development environment steps in CI #14025

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

Merged
merged 6 commits into from
Jun 30, 2023

Conversation

miketheman
Copy link
Member

Exercise some of our Makefile, docker-compose.yml and shell script wrapper utilities.

Originally conceived in #10052, simplified with current commands.

Closes #10052

Exercise some of our `Makefile`, `docker-compose.yml` and shell script
wrapper utilities.

Originally conceived in pypi#10052, simplified with current commands.

Closes pypi#10052

Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@miketheman miketheman requested a review from a team as a code owner June 26, 2023 20:01
@miketheman miketheman added testing Test infrastructure and individual tests developer experience Anything that improves the experience for Warehouse devs docker Pull requests that update Docker code labels Jun 26, 2023
Allow some time for the database to start up before performing "real"
readiness checks.

If the service is ready (healthcheck test command passes) during the
start_period, it'll be considered healthy and continue.

Refs: https://docs.docker.com/engine/reference/builder/#healthcheck:~:text=start%20period%20provides%20initialization%20time%20for%20containers%20that%20need,will%20be%20counted%20towards%20the%20maximum%20number%20of%20retries.

Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@miketheman
Copy link
Member Author

I don't know if this is a great idea yet.

It seems like a good idea, but it's also another ~12 minutes CI step.
Doesn't have to be required, but also maybe should run post-merge on main? Or some other time? Up for debate.

@di
Copy link
Member

di commented Jun 26, 2023

Doesn't have to be required, but also maybe should run post-merge on main?

Given that deploys are blocked on status checks passing, this might be a worse time to run it, unless we made it not required, which I'm not sure we'd ever see.

Instead, maybe we can run it regularly (daily?) via cron and alert us if it fails?

@miketheman
Copy link
Member Author

Given that deploys are blocked on status checks passing, this might be a worse time to run it, unless we made it not required, which I'm not sure we'd ever see.

Excellent point - definitely should not be in the critical path of anything, will remove those triggers.

Instead, maybe we can run it regularly (daily?) via cron and alert us if it fails?

I like this better. Have we established a notification pattern elsewhere? I have used Datadog CI before - I don't think we have that set up yet, but once it is, we can set a CI Monitor to tell us things we want to know.

Signed-off-by: Mike Fiedler <miketheman@gmail.com>
@di
Copy link
Member

di commented Jun 27, 2023

Have we established a notification pattern elsewhere?

Sort of, we do issue some Sentry messages like this:

sentry_sdk.capture_message(f"Attempted to store invalid ip_address: {e}")

I think that works fairly well? But definitely is made easier by the fact that we already have a Sentry integration present there.

@miketheman
Copy link
Member Author

... the fact that we already have a Sentry integration present there.

Yeah, that's during runtime, and bootstrapped the Sentry config already. I think with the schedule changes we can ship this, and figure out notification out of band.

@miketheman miketheman merged commit 59379b0 into pypi:main Jun 30, 2023
@miketheman miketheman deleted the miketheman/dev-env-test branch June 30, 2023 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
developer experience Anything that improves the experience for Warehouse devs docker Pull requests that update Docker code testing Test infrastructure and individual tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants