Issue/6549 add missing migration script #6596
Conversation
| async def test_type_change( | ||
| migrate_db_from: abc.Callable[[], abc.Awaitable[None]], get_tables_in_db: abc.Callable[[], abc.Awaitable[list[str]]] | ||
| ) -> None: | ||
| await migrate_db_from() |
There was a problem hiding this comment.
I would have liked to assert that the type is int before the migration but I didn't manage to
There was a problem hiding this comment.
I would expect you just need to same code as after the migrate_from_db() call but with str replaced by int?
There was a problem hiding this comment.
The old way of checking was raising an exception: calling this before the migrate_from_db() call would fail because of db pool exhaustion
env = await Environment.get_one(name="dev-1")
assert isinstance(env.settings["autostart_agent_deploy_interval"], int)
assert isinstance(env.settings["autostart_agent_repair_interval"], int)There was a problem hiding this comment.
Do you know why we got the pool exhaustion. This is very strange.
There was a problem hiding this comment.
I find it strange as well, I have no idea
| async def test_type_change( | ||
| migrate_db_from: abc.Callable[[], abc.Awaitable[None]], get_tables_in_db: abc.Callable[[], abc.Awaitable[list[str]]] | ||
| ) -> None: | ||
| await migrate_db_from() |
There was a problem hiding this comment.
I would expect you just need to same code as after the migrate_from_db() call but with str replaced by int?
|
Do we also have a test somewhere that verifies that getting these settings via the API returns data in the right type? |
| async def test_type_change( | ||
| migrate_db_from: abc.Callable[[], abc.Awaitable[None]], get_tables_in_db: abc.Callable[[], abc.Awaitable[list[str]]] | ||
| ) -> None: | ||
| await migrate_db_from() |
There was a problem hiding this comment.
Do you know why we got the pool exhaustion. This is very strange.
Added the |
tests/server/test_env_settings.py
Outdated
| @@ -38,6 +38,17 @@ def check_only_contains_default_setting(settings_dict: Dict[str, object]) -> Non | |||
| assert setting_value == get_environment_setting_default(setting_name) | |||
|
|
|||
|
|
|||
| async def test_api_return_type(client, server, environment_default): | |||
There was a problem hiding this comment.
Why does this test case use the environment_default fixture instead of the environment fixture we always use?
There was a problem hiding this comment.
Ah, I copy pasted the test definition from test_environment_settings. Fixed to use the environment fixture.
I wonder, what would the issue be with using environment_default instead ? I'm guessing it is the fact that auto_deploy is true in that one ? Could it cause flakiness/slowness ?
|
Processing this pull request |
# Description Previous PR was missing a migration script closes #6549 # Self Check: Strike through any lines that are not applicable (`~~line~~`) then check the box - [ ] Attached issue to pull request - [ ] Changelog entry - [ ] Type annotations are present - [ ] Code is clear and sufficiently documented - [ ] No (preventable) type errors (check using make mypy or make mypy-diff) - [ ] Sufficient test cases (reproduces the bug/tests the requested feature) - [ ] Correct, in line with design - [ ] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: ) - [ ] If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)
|
Merged into branches master in ba616b3 |
# Description Previous PR was missing a migration script closes #6549 # Self Check: Strike through any lines that are not applicable (`~~line~~`) then check the box - [ ] Attached issue to pull request - [ ] Changelog entry - [ ] Type annotations are present - [ ] Code is clear and sufficiently documented - [ ] No (preventable) type errors (check using make mypy or make mypy-diff) - [ ] Sufficient test cases (reproduces the bug/tests the requested feature) - [ ] Correct, in line with design - [ ] End user documentation is included or an issue is created for end-user documentation (add ref to issue here: ) - [ ] If this PR fixes a race condition in the test suite, also push the fix to the relevant stable branche(s) (see [test-fixes](https://internal.inmanta.com/development/core/tasks/build-master.html#test-fixes) for more info)
|
Processing #6605. |
Description
Previous PR was missing a migration script
closes #6549
Self Check:
Strike through any lines that are not applicable (
~~line~~) then check the box