-
Notifications
You must be signed in to change notification settings - Fork 56
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
Move from Buildkite to GitHub Actions, attempt 2 #1127
Conversation
bootstrap script is in the sytest-* containers, and I was running their version instead of the version in the checkout. Confusing!!
Getting somewhere
that don't work son
was just to force actions to trigger in my fork.
Co-authored-by: Andrew Morgan <[email protected]>
Need to ensure that API and POSTGRES are the empty strings for dendrite when they're meant to be off. But ${{ matrix.api == 'full-http' && foo }} seems to yield either foo or `false`. Stick with what we know works.
but only on BUILDKITE. Keep this around until we're confident with the GHA workflow.
- Don't use API for synapse; use right matrix key for WORKERS - echo env vars before we invoke bootstrap script
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.
(Interestingly the actions runs seem to have appeared on the other PR as well).
Looking through the test results I think they look good! Some test failures, but they look like flakes - possibly an outdated Sytest branch?
Carrying over from the old PR, I think there's only minor tweaks. Otherwise this looks fine to merge to me.
- label: Postgres | ||
postgres: postgres | ||
|
||
- label: Postgres, full HTTP APIs | ||
postgres: postgres | ||
api: full-http |
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 suppose we don't have a clear idea of postgres versions here as there's no distribution label for sytest-dendrite
?
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.
sytest-dendrite:latest
looks to be based on Debian buster at the time of writing according to docker/build.sh:16
. I was a bit hesitant to write in a PG version for a tag that could change under our feet. But we already do this for sytest-synapse:testing
, based on sytest:testing
, based on debian:testing
.
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.
Good point, yeah. Actually, I suppose it might be a good idea to simply remove the version of debian:testing
as well. I can't see us really remembering to ever update that version number...
But one can check what it is if they really need to.
Migrated from #1115 after I screwed things up by deleting a personal fork.
On the plus side, sticking it in this repo should mean that we can see the Actions directly here.