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

ci: split nix-build-test into nix-build nix-test and introduce circle ci contexts #1817

Merged
merged 4 commits into from
Apr 16, 2021

Conversation

wolfgangwalther
Copy link
Member

As discussed via email. This is to secure our cachix, docker and github tokens better. Contexts will still have to be filled with environment variables.

The split in nix-build and nix-test should also make our pipeline faster, because they can both run in parallel. @monacoremo Did I miss anything in there? I don't think the nix-build step will have any problems, but will that be OK for nix-test? Or is it missing anything that I removed in that step?

@wolfgangwalther
Copy link
Member Author

(I cancelled the nix steps for now, because the cache is not built up again, yet)

@wolfgangwalther
Copy link
Member Author

Just noticed that we seem to have a limit of 4 jobs running in parallel on circle. Not too much of a problem because style-check is done pretty quickly. Still I wondered whether we should move the two stack- jobs into one, maybe. Or if we'd just kill the stack-test-memory one. We're not running the full test suite (no io tests) anyway with stack, so maybe the basic spec test is enough for testing basic functionality of the build that was made with stack?

@steve-chavez
Copy link
Member

Or if we'd just kill the stack-test-memory one.

👍 For that. A single stack job should be enough, we just need to make sure it builds(so stackage can notify robx of dependency changes).

@wolfgangwalther
Copy link
Member Author

@monacoremo nix-build on the first commit of this PR was running for > 1h so far: https://app.circleci.com/pipelines/github/PostgREST/postgrest/911/workflows/f1b6b8cf-2f66-486f-8811-dcb3ac232bfb/jobs/8737. I cancelled it now, because it would not push anything to cachix anyway.

Are we still missing some of the cache?

@monacoremo
Copy link
Member

Yes, noticed that some of the static build assets were not pushed yet. Running that now

@wolfgangwalther
Copy link
Member Author

Also added a checksum check for the codecov bash script and reordered nix-test a bit to report coverage back earlier.

The tokens are still going to be available through regular environment variables for now. @steve-chavez once you have added the new tokens to the contexts, you can remove the old ones from the env vars. I suggest you then trigger a nightly build - that should test whether context + tokens work fine.

Since we can't test it in the PR anyway, because the tokens won't be used here, I'm going to merge this now.

@wolfgangwalther
Copy link
Member Author

Since we can't test it in the PR anyway, because the tokens won't be used here, I'm going to merge this now.

Actually, I will wait for @monacoremo to build the remaining cache and then merge to avoid a long running build job on main. Let me know!

@wolfgangwalther wolfgangwalther merged commit 1f206a5 into PostgREST:main Apr 16, 2021
@wolfgangwalther wolfgangwalther deleted the ci/circle-context branch April 16, 2021 12:10
gautam1168 pushed a commit to gautam1168/postgrest that referenced this pull request Aug 18, 2021
This fixes issue PostgREST#1817, Add Retry-After hint when in recovery mode
steve-chavez pushed a commit that referenced this pull request Aug 18, 2021
Add Retry-After header when response status is 503.
Its value is the connection worker delay(seconds) when
it's recovering.

This closes issue #1817.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants