chore(ci): configure CircleCI for postgres builds#1840
Conversation
a630225 to
e87e670
Compare
79f9a21 to
0513c7b
Compare
| name: Install MySQL client | ||
| command: sudo apt-get update && sudo apt-get install -y default-mysql-client | ||
| create-tokenserver-database: | ||
| create-tokenserver-database-mysql: |
There was a problem hiding this comment.
Naming the jobs more explicitly since we are introducing another db into the mix.
| type: string | ||
| steps: | ||
| - run: | ||
| name: Compress and copy coverage data |
There was a problem hiding this comment.
This doesn't save time when saving data but does save a little bit of time when attaching workspace later.
| RUST_BACKTRACE: 1 | ||
| # XXX: begin_test_transaction doesn't play nice over threaded tests | ||
| RUST_TEST_THREADS: 1 | ||
| - image: cimg/postgres:18.0 |
There was a problem hiding this comment.
I don't remember any discussion on postgres versions.
There was a problem hiding this comment.
We can do the same thing with pinning a Postgres version like we will with Spanner, but explicitly defining 18 like you have here is perfect
| if [ "$TOKENSERVER_DATABASE_BACKEND" = "postgres" ]; then \ | ||
| TOKENSERVER_FEATURES="--features=tokenserver-db/postgres"; \ | ||
| fi && \ | ||
| cargo chef cook --release --no-default-features --features=syncstorage-db/$SYNCSTORAGE_DATABASE_BACKEND $TOKENSERVER_FEATURES --features=py_verifier --recipe-path recipe.json |
There was a problem hiding this comment.
Temporarily support the permutation of Spanner Syncserver and Postgres Tokenserver while we await for Syncserver Postgres support.
| if [ "$TOKENSERVER_DATABASE_BACKEND" = "postgres" ]; then \ | ||
| TOKENSERVER_FEATURES="--features=tokenserver-db/postgres"; \ | ||
| fi && \ | ||
| cargo chef cook --release --no-default-features --features=syncstorage-db/$SYNCSTORAGE_DATABASE_BACKEND $TOKENSERVER_FEATURES --features=py_verifier --recipe-path recipe.json |
There was a problem hiding this comment.
Temporarily support the permutation of Spanner Syncserver and Postgres Tokenserver while we await for Syncserver Postgres support.
| # update again now that we trust repo.mysql.com | ||
| apt-get -q update && \ | ||
| apt-get -q install -y build-essential $MYSQLCLIENT_PKG libssl-dev libffi-dev libcurl4 python3-dev python3-pip python3-setuptools python3-wheel python3-venv cargo curl jq pkg-config && \ | ||
| apt-get -q install -y build-essential $MYSQLCLIENT_PKG $POSTGRES_PKG libssl-dev libffi-dev libcurl4 python3-dev python3-pip python3-setuptools python3-wheel python3-venv cargo curl jq pkg-config && \ |
There was a problem hiding this comment.
Both MySQL and Postgres packages are installed because the Python based integration tests have an implicit dependency on MySQL.
There was a problem hiding this comment.
Yeah, same issue with Spanner needing SQL, as we unfortunately learned with the public key fiasco...
eb5a174 to
14b0f6b
Compare
| AND downed = 0 | ||
| AND backoff = 0 | ||
| ORDER BY LOG(current_load) / LOG(capacity) | ||
| ORDER BY CASE WHEN current_load = 0 THEN -1 ELSE LN(current_load) / LN(capacity) END |
There was a problem hiding this comment.
(The changes in this file are not directly CI related but they are needed for the Tokenserver unit tests on Postgres to pass.)
When LOG(0) is called in MySQL it returns a NULL along with a warning, and that behavior along with how MySQL's ORDER BY treats NULL made that query work. It does not work in Postgres. Postgres throws an error when the query tries to take the LOG of 0.
| AND email = $4 | ||
| AND generation <= $5 | ||
| AND COALESCE(keys_changed_at, 0) <= COALESCE(<keys_changed_at i64>, keys_changed_at, 0) | ||
| AND COALESCE(keys_changed_at, 0) <= COALESCE($6, keys_changed_at, 0) |
There was a problem hiding this comment.
I think it was just a good ol' bug.
| params: params::GetOrCreateUser, | ||
| ) -> Result<results::GetOrCreateUser, DbError> { | ||
| TokenserverPgDb::get_or_create_user(self, params).await | ||
| } |
There was a problem hiding this comment.
Infinite call loop from trait impl delegation.
There was a problem hiding this comment.
Good call. This was missed when we moved the get_or_create_user call into tokenserver-db-common since it was identical for MySQL and Postgres.
|
I'm considering this ready since the workflow is there. We'll update the jobs and commands as we progress with the Postgres work. |
fb59533 to
ad11ea4
Compare
| hawkauthlib = "^2.0.0" | ||
| konfig = "^1.1" | ||
| mysqlclient = "^2.2.7" | ||
| psycopg2-binary = "^2.9.11" |
There was a problem hiding this comment.
Either works for me, I'll add them regardless in #1857 along with the poetry lockfile, so both should end up looking the same.
taddes
left a comment
There was a problem hiding this comment.
👍 lgtm, I'll review again after the rebase for Dockerfile we discussed.
| params: params::GetOrCreateUser, | ||
| ) -> Result<results::GetOrCreateUser, DbError> { | ||
| TokenserverPgDb::get_or_create_user(self, params).await | ||
| } |
There was a problem hiding this comment.
Good call. This was missed when we moved the get_or_create_user call into tokenserver-db-common since it was identical for MySQL and Postgres.
| hawkauthlib = "^2.0.0" | ||
| konfig = "^1.1" | ||
| mysqlclient = "^2.2.7" | ||
| psycopg2-binary = "^2.9.11" |
There was a problem hiding this comment.
Either works for me, I'll add them regardless in #1857 along with the poetry lockfile, so both should end up looking the same.
ad11ea4 to
6869436
Compare
a1bd24b to
45f3027
Compare
| # SQLAlchemy 1.4+ wants postgresql | ||
| if db_url.startswith("postgres://"): | ||
| db_url = db_url.replace("postgres://", "postgresql://", 1) | ||
| engine = create_engine(db_url) |
There was a problem hiding this comment.
Here’s hoping it works 😅🤞🍀
50e111d to
a6fe88e
Compare
This commit enables Postgres builds and tests in CircleCI. Only Tokenserver supports Postgres at the moment, and the Python based integration tests have an implicit dependency on MySQL, so the CI jobs only use Postgres where possible. The jobs have been split up and named more explicitly. The hope is to simply delete the mysql jobs in the future.
b895ede to
3047868
Compare
Upstream split the DATABASE_BACKEND parameter/var into SYNCSTORAGE_DATABASE_BACKEND and TOKENSERVER_DATABASE_BACKEND in mozilla-services/syncstorage-rs#1840
Upstream split the DATABASE_BACKEND parameter/var into SYNCSTORAGE_DATABASE_BACKEND and TOKENSERVER_DATABASE_BACKEND in mozilla-services/syncstorage-rs#1840
Description
This commit enables Postgres builds and tests in CircleCI. Only
Tokenserver supports Postgres at the moment, and the Python based
integration tests have an implicit dependency on MySQL, so the CI jobs only
use Postgres where possible.
The jobs have been split up and named more explicitly. The hope is to
simply delete the mysql jobs in the future.
Testing
The Tokenserver unit tests running on Postgres should pass.
Issue(s)
Closes STOR-353 / #1791