Use nox to test different environments #1245

medihack · 2024-11-25T11:29:30Z

I started integrating nox to solve our problem with the different environments (latest stable and current version). This is not finished yet, but I have a plan for how to do it.
I also added a custom option to pytest named --skip-post-migration to skip the last post-migration file when setting up the database. But this reveals more problems with my pre-/post-migrations that I understand but don’t know how to solve. The main problem is that without the post-migration we are already using procrastinate_job_status_v1 in the current version on multiple occasions (for example, in queries.sql), but the procrastinate_jobs table still uses procrastinate_job_status (without “_v1”) as this conversion is part of the post-migration. But there is no way to do this conversion in the pre-migration as this would break the latest stable version 😞.

github-actions · 2024-11-25T19:48:11Z

Coverage report

Click to see where and how coverage changed

File	Statements	Missing	Coverage	Coverage (new stmts)	Lines missing
procrastinate
app.py
cli.py
connector.py
job_context.py
jobs.py
manager.py
metadata.py
periodic.py
psycopg_connector.py
retry.py
sync_psycopg_connector.py
testing.py
types.py
utils.py
worker.py
procrastinate/contrib/aiopg
aiopg_connector.py
procrastinate/contrib/django
apps.py
django_connector.py
migrations_utils.py
procrastinate/contrib/psycopg2
psycopg2_connector.py
procrastinate/contrib/sqlalchemy
psycopg2_connector.py
Project Total

_{This report was generated by python-coverage-comment-action}

medihack · 2024-11-25T19:52:12Z

Ouch, this was quite nasty 🤪. I now went another route by keeping procrastinate_job_status and procrastinate_job_event_type unversioned. I also kept the aborting status, but indicated that it is unused. We can remove it later on (if necessary). At least I now have a working pre- and post-migration setup with the current version. I am already scared of trying the latest stable version with pre-migration 🫣. nox is not yet used during CI, upcoming.

ewjoachim · 2024-11-26T12:54:37Z

tests/conftest.py

+        migrations = sorted(migrations_path.glob("*.sql"))
+        last_migration = migrations[-1]
+        if "_post_" in last_migration.name:
+            migrations.remove(last_migration)


This is not right.
1/ There may be multiple post_migrations
2/ maybe the last release was 3.4, the current release we're working on will be 3.5, and 3.4 had no migrations and the last migrations are the ones for 3.3, and in that case we don't want to test 3.5 without the post-migrations of 3.3. The only post-migrations we want to skip are those that haven't been released yet.

Sure. Sorry, I should have made this a draft. It's just temporary, but I think I will fetch the latest tag in the nox session and then provide it as an option to pytest (seems to me more like part of the environment).

medihack · 2024-11-26T23:09:52Z

@ewjoachim I finally got it working. Some tests of nox -s stable_version_without_post_migration still fail, but that's expected as I run the current tests with the old stable version. I wonder how we deal with those, or what the idea is here. Wouldn't it make more sense to run the old tests (by cloning the old code) with the new migrations (up until "pre")?

ewjoachim · 2024-12-01T13:29:37Z

noxfile.py

+        shutil.copytree(base_path / "tests", temp_path / "tests")
+        shutil.copy(base_path / "pyproject.toml", temp_path / "pyproject.toml")
+        shutil.copy(base_path / "poetry.lock", temp_path / "poetry.lock")
+        os.chdir(temp_dir)


https://docs.pytest.org/en/7.1.x/reference/reference.html#pytest.MonkeyPatch.chdir so it will be revrted at the end of the test

EDIT: ah sorry I was thinking pytest but this is a Nexfile. Plus, you're fixing it in the next commit.

ewjoachim · 2024-12-01T13:39:40Z

noxfile.py

+
+
+@nox.session
+def stable_version_without_post_migration(session: nox.Session):


It's funny because in the end the noxfile solution ended up being not that far from my original solution. Of course, it's an excellent idea to use nox so that we don't have to bother with creating our venvs ourselves. But in the end, if you want to be sure to run exactly the tests you want with the procrastinate you want, you need a tmpdir, and run things inside.

In my solution, we use the same pytest process for everything, and it calls subprocesses for procrastinate. Whereas in your solution, we call distinct pytest subprocesses for each of the 3 cases.

In my solution, we don't need to install anything else than procrastinate in the venv, whereas in your solution, we install pytest in all venvs.

In your solution, we keep existing acceptance tests. In my solution, we need to rewrite them.

I don't think any approach outshines the other one at 100%, and I'd be comfortable with either. We're just not going to end up with the same set of problems.

ewjoachim

We need to adapt the CI of course. Otherwise, I'm looking forward to discussing this with you in pair :)

ewjoachim · 2024-12-01T13:41:39Z

docs/howto/production/migrations.md

+-   `xx.yy.zz` is the version of Procrastinate the migration script can be applied to.
+-   `ab` is the migration script's serial number, `01` being the first number in the
+    series.
+-   `pre` / `post`: indicates wether the migration should be applied before


Suggested change

- `pre` / `post`: indicates wether the migration should be applied before

- `pre` / `post`: indicates whether the migration should be applied before

(my own mistake)

ewjoachim

Running locally: yes, I think we end up having problems due to tests acceptance added by 3.x not compatible with 2.x :/

ewjoachim · 2024-12-01T13:42:19Z

docs/howto/production/migrations.md

+    series.
+-   `pre` / `post`: indicates wether the migration should be applied before
+    upgrading the code (`pre`) or after upgrading the code (`post`) in the context
+    of a blue-green deployment. On old migrations, if `pre` or `post` is not


Suggested change

of a blue-green deployment. On old migrations, if `pre` or `post` is not

of a blue-green deployment (see below). On old migrations, if `pre` or `post` is not

ewjoachim · 2024-12-01T13:42:49Z

docs/howto/production/migrations.md

+$ ...
+```
+
+3. Upgrade your code to the new Procrastinate version.
 4. Start all the services.

 This, as you've noticed, only works if you're able to stop the services.

 ## The safer way, without service interruption


Suggested change

## The safer way, without service interruption

## The safer way, without service interruption (a.k.a "blue-green")

ewjoachim · 2024-12-01T13:48:29Z

docs/howto/production/migrations.md

-you will have to interrupt the service or write custom migrations.
-:::
+If you need to ensure service continuity, you'll need to make intermediate upgrades.
+Basically, you'll need to stop at every version that provides migrations.


Suggested change

Basically, you'll need to stop at every version that provides migrations.

Basically, you'll need to stop at every version that provides migrations.

Repeat the following for all versions:

1. Apply pre-migrations

2. Deploy version to your system

3. Apply post-migrations

4. Repeat until you reach the desired version

If multiple versions in a row don't have migrations, you can deploy the last of them without going through intermediate ones. Pre-migrations are only guaranteed to work on the immediately previous version. Post-migrations are only guaranteed to work on their own version.

ewjoachim · 2024-12-01T14:46:17Z

noxfile.py

+        shutil.copy(base_path / "pyproject.toml", temp_path / "pyproject.toml")
+        shutil.copy(base_path / "poetry.lock", temp_path / "poetry.lock")


If we export the deps, why do we need pyproject.toml ? (and vice versa)

ewjoachim · 2024-12-01T14:51:45Z

tests/utils.py

I think this file is now unused?

…G status just yet

ewjoachim · 2024-12-01T20:43:22Z

CONTRIBUTING.md

@medihack this is the file where most of the doc we discussed was added

ewjoachim · 2024-12-01T21:29:21Z

Congratulations @medihack !

medihack requested a review from a team as a code owner November 25, 2024 11:29

medihack mentioned this pull request Nov 25, 2024

Rework migrations #1237

Closed

10 tasks

ewjoachim reviewed Nov 26, 2024

View reviewed changes

medihack marked this pull request as draft November 26, 2024 16:41

ewjoachim reviewed Dec 1, 2024

View reviewed changes

ewjoachim force-pushed the use-nox-for-different-environments branch 3 times, most recently from 50aba0c to c347eed Compare December 1, 2024 19:08

ewjoachim added 8 commits December 1, 2024 21:41

Upgrade deps

5694f86

New Migrations procedure: documentation

05f5a02

Cosmetics - just pre-commit reorganizing things

7e9fc8e

Make nominal test less flaky

ecc7fc5

Rework migrations, queries, version SQL objects, don't remove ABORTIN…

d34af17

…G status just yet

Remove coverage by default in pytest

e49b5d5

New acceptance tests system: running 3 times

3c0bc03

Skip recent acceptance tests on 2.x

0e83c30

ewjoachim force-pushed the use-nox-for-different-environments branch from 11b94c9 to da7e37b Compare December 1, 2024 20:41

ewjoachim marked this pull request as ready for review December 1, 2024 20:42

ewjoachim reviewed Dec 1, 2024

View reviewed changes

CONTRIBUTING.md Outdated

Copy link

Member

ewjoachim Dec 1, 2024

Choose a reason for hiding this comment

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

@medihack this is the file where most of the doc we discussed was added

ewjoachim force-pushed the use-nox-for-different-environments branch from da7e37b to 05e182b Compare December 1, 2024 21:20

Try simplifying the noxfile

355da01

ewjoachim force-pushed the use-nox-for-different-environments branch from 05e182b to 355da01 Compare December 1, 2024 21:26

ewjoachim merged commit 6df6867 into v3 Dec 1, 2024
13 checks passed

ewjoachim deleted the use-nox-for-different-environments branch December 1, 2024 21:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use nox to test different environments #1245

Use nox to test different environments #1245

medihack commented Nov 25, 2024 •

edited

Loading

github-actions bot commented Nov 25, 2024 •

edited

Loading

medihack commented Nov 25, 2024 •

edited

Loading

ewjoachim Nov 26, 2024

medihack Nov 26, 2024

medihack commented Nov 26, 2024 •

edited

Loading

ewjoachim Dec 1, 2024 •

edited

Loading

ewjoachim Dec 1, 2024

ewjoachim left a comment

ewjoachim Dec 1, 2024 •

edited

Loading

ewjoachim left a comment

ewjoachim Dec 1, 2024

ewjoachim Dec 1, 2024

ewjoachim Dec 1, 2024

ewjoachim Dec 1, 2024

ewjoachim Dec 1, 2024

ewjoachim Dec 1, 2024

ewjoachim commented Dec 1, 2024



		@nox.session
		def stable_version_without_post_migration(session: nox.Session):

	- `pre` / `post`: indicates wether the migration should be applied before
	- `pre` / `post`: indicates whether the migration should be applied before

	of a blue-green deployment. On old migrations, if `pre` or `post` is not
	of a blue-green deployment (see below). On old migrations, if `pre` or `post` is not

	## The safer way, without service interruption
	## The safer way, without service interruption (a.k.a "blue-green")

-Basically, you'll need to stop at every version that provides migrations.
+Basically, you'll need to stop at every version that provides migrations.
+Repeat the following for all versions:
+. Apply pre-migrations
+. Deploy version to your system
+. Apply post-migrations
+. Repeat until you reach the desired version
+If multiple versions in a row don't have migrations, you can deploy the last of them without going through intermediate ones. Pre-migrations are only guaranteed to work on the immediately previous version. Post-migrations are only guaranteed to work on their own version.

		shutil.copy(base_path / "pyproject.toml", temp_path / "pyproject.toml")
		shutil.copy(base_path / "poetry.lock", temp_path / "poetry.lock")

Use nox to test different environments #1245

Use nox to test different environments #1245

Conversation

medihack commented Nov 25, 2024 • edited Loading

github-actions bot commented Nov 25, 2024 • edited Loading

Coverage report

medihack commented Nov 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

medihack commented Nov 26, 2024 • edited Loading

ewjoachim Dec 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewjoachim left a comment

Choose a reason for hiding this comment

ewjoachim Dec 1, 2024 • edited Loading

Choose a reason for hiding this comment

ewjoachim left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewjoachim commented Dec 1, 2024

medihack commented Nov 25, 2024 •

edited

Loading

github-actions bot commented Nov 25, 2024 •

edited

Loading

medihack commented Nov 25, 2024 •

edited

Loading

medihack commented Nov 26, 2024 •

edited

Loading

ewjoachim Dec 1, 2024 •

edited

Loading

ewjoachim Dec 1, 2024 •

edited

Loading