Update to Python 3.11 #193
Conversation
| return process | ||
|
|
||
| config_file = make_config_file({'queues': {'some-queue': {'handler': 'azafea.tests.test_cli'}}}) | ||
| config_file = make_config_file({'queues': {'some-queue': {'handler': 'azafea.tests'}}}) |
There was a problem hiding this comment.
I don't understand this change.
tests: Make test queue handlers a package
Azafea is setup to expect the handler methods (
processand
register_commands) in a package withmigrationsas a sub-package.
Otherwise newer alembic blows up when it tries to look for the
migrationssub-package when it's the handler is defined as a module.
In this commit you're just changing these config files but you're not making any changes to azafea.tests. And that namespace doesn't define those two functions or the sub-package migrations. How can this work / do anything?
There was a problem hiding this comment.
I wrote that commit message hastily. What's happening is get_callable is being mocked just above that so that it magically returns a process method without having to add one to the actual package/module specified in handler. In essence, this is a fake handler and what's being specified doesn't matter except that it's an importable path. Most of the cases I think would be fine left as is, but I changed them all to be consistent since specifying a leaf module instead of a package would not be legitimate.
Where it actually matters is when alembic gets involved via make-migration or migratedb. In that case, we dynamically tell alembic that the migrations directories (as configured with version_locations) are relative to the handler module. If the "module" isn't a directory (i.e., a package), then alembic (or maybe our code driving alembic) fails.
There was a problem hiding this comment.
If I'm being honest, I didn't really understand the issue completely until I wrote that comment.
| def test_make_empty_migration(self): | ||
| self.run_subcommand('make-migration', 'test_make_empty_migration') | ||
|
|
||
| migrations = (self._handler_path / 'migrations').listdir() |
There was a problem hiding this comment.
AFAICT pathlib.Path never had a .listdir() method – how did this work before?!
There was a problem hiding this comment.
Because it used to be a tmpdir fixture, which returns a py.path.local object. I've never looked closely, but I think it's basically a precursor to pathlib.Path.
|
Looks good except for the documentation building test failing – will the same warning prevent publication on Rtd? Or we could merge & start testing the code changes in dev. |
This instructs pipenv to sort entries when adding packages with `install`. The lists were mostly sorted already, but it's nice to have the tool honor that choice. This requires pipenv 2023.10.24.
There's no reason we need pytest to be driving this and it results in less usable output. More importantly, it allows updating flake8 as pytest-flake8 uses internal flake8 API and does not work with recent versions of flake8.
Prior to mypy 1.0, `Optional` would be implicitly added to any argument that defaulted to `None`. Add them explicitly to avoid errors on later mypy upgrade.
Pydantic 2 changes requires migration[1]. For now, pin to Pydantic 1 to keep using the existing syntax. Unfortunately, Pydantic 1.9 introduces a new type for error details that breaks our type checking. Also unfortunately, Pydantic 1.8.2 breaks with mypy 0.920, so also pin that. Both will be cleaned up in a subsequent update to Pydantic 2. 1. https://docs.pydantic.dev/latest/migration/
Azafea is setup to expect the handler methods (`process` and `register_commands`) in a package with `migrations` as a sub-package. Otherwise newer alembic blows up when it tries to look for the `migrations` sub-package when it's the handler is defined as a module.
When using python >= 3.9, coerce_resource_to_filename returns a Path rather than a str since it uses importlib.resources.as_file[1] rather than importlib_resources.as_file[2]. Ensure a str is always returned as that's what the rest of the code expects. 1. https://docs.python.org/3/library/importlib.resources.html#importlib.resources.as_file 2. https://importlib-resources.readthedocs.io/en/latest/api.html#importlib_resources.as_file
The azafea configuration expects that handlers are packages with migration directories within them. Several integration tests using their own handlers weren't following that convention and fail with newer alembic. In order to make them work in that fashion, make a temporary package with the handler module code copied into it. Since the migrations are in a temporary directory, all of that cleanup goes away. For `IntegrationTest`, allow setting a handler module file path to trigger the temporary handler package mode. Tests that need to import the handler module need to do so from the copied path so that sqlalchemy doesn't see duplicated tables. While here, add some logging to `IntegrationTest` since it does a lot of very non-trivial things and the messages make it easier to pinpoint where things are going wrong. This also uses `monkeypatch.syspathprepend` for tweaking `sys.path` and the `tmp_path` fixture rather than `tmpdir`. `syspathprepend` allows a `FIXME` to go away as it has a `pkg_resources` workaround. `tmp_path` is basically the same as `tmpdir`, but the former uses a regular `pathlib.Path` and the latter is deprecated.
With python >= 3.10.7 and mypy < 0.981[1], type definitions noting positional only parameters break mypy[2]. Since mypy has been pinned below 0.920 for pydantic, pin redis and requests to their current versions until after the pydantic migration. 1. python/mypy#13627 2. python/typeshed#11554
No bash features are used and this avoids an issue where bash isn't installed by default on newer alpine releases.
Disable typehints in function signatures. Autodoc is only being used to document events and the types used in event initialization is irrelevant. The types can't always be found by sphinx, which causes warnings in newer versions. Those warnings become errors with `--fail-on-warning`.
This matches what's currently used in Endless OS and Debian stable. Note that specifying `python_version` in `Pipfile` will cause pipenv to try to use Python 3.11 if it's available. It will fallback to the default Python on the system with a warning if that's not available.
With the pins in place, update all the dependencies so they're separated from any explicit upgrades.
The pydantic usage here is pretty light, so the changes needed aren't that dramatic. * The dataclass `__post_init_post_parse__` method is no longer supported and simply `__post_init__` should be used. * `TypeError` is no longer converted to `ValidationError` in validators, so just use `ValueError`. * The `@validator` decorator has changed to `@field_decorator` with a minor change in parameters. * The `@root_validator` decorator has changed to `@model_validator`. However, that doesn't actually work in the case here where it's being used to generate values for dataclass fields with `init=False`. Instead, use a `__post_init__` method for the value generation. That also changes the error `loc` field tweaking a bit since the final module value is no longer `__root__` as would happen with `@root_validator`. * Validator error messages now include the error type, which means `Value error` for all of our validators. * The `ValidationError.errors()` detail list now has a custom `ErrorDetail` type. This exposes the fact that the `loc` attribute is actually a tuple of `str` and `int`. Mypy flags that as an issue if an `int` is passed to `str.join`. We don't actually have any cases where an `int` would occur, but copy a converter from the documentation so it's handled correctly in case that does happen. This also allows dropping the mypy upper version pin as updated Pydantic contains the necessary compatibility fix. https://docs.pydantic.dev/2.7/migration/
Now that Pydantic has been updated and the mypy pin has been removed, update redis and requests. For redis in particular, it should be possible to drop types-redis now, but there are some issues with redis-py's native types that will need a bit of work.
This supersedes #186 and is heavily inspired by endlessm/azafea-metrics-proxy#27. Most of the changes were directly taken from that PR, but a few more changes were needed here since there's postgres usage and a slightly complex config setup for
queues.After this, only sqlalchemy isn't current (see #147) and it would be nice to drop types-redis but was a little more complex than I expected.