chore(databases): clarify fork naming behavior - #181
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| ### Changed | ||
|
|
||
| - chore(databases): clarify fork naming behavior |
There was a problem hiding this comment.
Blocking: the only changelog entry for this PR is the fork-naming doc tweak, but the regeneration also deletes a large slice of the public surface:
SecretsApi,RefreshApi,ConnectionTypesApi(dropped fromhotdata/api/__init__.pyandhotdata/__init__.py)- ~20 models (
CreateSecretRequest,GetSecretResponse,RefreshRequest/RefreshResponse,ConnectionHealthResponse,ConnectionTypeDetail,SchemaRefreshResult, …) ConnectionsApi.check_connection_health(seedocs/ConnectionsApi.md, −87)JobTypemembers (the refresh-related variants) and the refresh variants of theJobResultoneOf
Per the repo's own SemVer policy — and the precedent set by the 0.10.0 entry — this needs a ### Removed block flagged **Breaking:** describing what callers lose and what to use instead, not a single chore(databases) line under ### Changed.
Also worth confirming this removal is intentional: the PR title/description only mention fork naming, so a spec that silently dropped three whole tag groups is worth double-checking before merge.
There was a problem hiding this comment.
Review
Note on context: gh pr diff fails on this PR (HTTP 406: diff exceeded the maximum number of files (300)) and the prompt's full-diff block was empty, so I reviewed by reading the post-merge checkout: CHANGELOG.md, hotdata/__init__.py, hotdata/api/__init__.py, hotdata/models/fork_database_request.py, hotdata/models/job_type.py, hotdata/models/job_result.py, hotdata/api/connections_api.py, README.md, the tests/integration/ suite, and the two CI workflow files. The intended change itself (the ForkDatabaseRequest.name description) is fine.
Blocking Issues
1. tests/integration/conftest.py imports three modules this PR deletes — the whole integration suite fails at collection.
tests/integration/conftest.py:17—from hotdata.api.connection_types_api import ConnectionTypesApitests/integration/conftest.py:25—from hotdata.api.refresh_api import RefreshApitests/integration/conftest.py:27—from hotdata.api.secrets_api import SecretsApi
All three modules are removed by this PR. These are module-scope imports in conftest.py, so every test under tests/integration/ errors on collection, not just the ones using the fixtures (secrets_api at :155, connection_types_api at :170, refresh_api at :190). The unit job passes --ignore=tests/integration so it is unaffected; the integration job (pytest tests/integration -v, .github/workflows/integration-tests.yml:99) is not.
2. Two scenario tests import deleted modules directly.
tests/integration/test_secrets_crud.py:15,17,18—hotdata.api.secrets_api,hotdata.models.create_secret_request,hotdata.models.update_secret_requesttests/integration/test_connection_types_read.py:9—hotdata.api.connection_types_api
3. CHANGELOG does not record the removals. Details in the inline comment on CHANGELOG.md:12.
Action Required
- Delete
tests/integration/test_secrets_crud.pyandtests/integration/test_connection_types_read.py, and drop thesecrets_api/connection_types_api/refresh_apifixtures and their imports fromtests/integration/conftest.py. - Add a Removed / Breaking entry to the
[Unreleased]section covering the removed API classes, models,ConnectionsApi.check_connection_health, and the droppedJobType/JobResultvariants. - Confirm the endpoint removals are intended by the upstream spec change rather than an artifact of the regeneration, since the PR is described as a docs-only clarification.
Non-blocking:
- nit:
tests/integration/test_connections_read.py:31callsconnections_api.check_connection_health(...), which no longer exists. The test is@pytest.mark.skip-ed so it will not fail today, but it willAttributeErrorwhenever the seeded-connection fixture is restored and the skip is lifted. (not blocking) - super nit:
README.md:3still listssecretsamong the covered API areas. (not blocking)
…ectionTypes APIs The regeneration removes hotdata.api.secrets_api, hotdata.api.refresh_api, hotdata.api.connection_types_api, ConnectionsApi.check_connection_health, and their models, but the hand-written integration suite still imported them, so every test failed at collection. - drop the secrets_api / refresh_api / connection_types_api fixtures and imports from tests/integration/conftest.py - delete test_secrets_crud.py and test_connection_types_read.py; drop the check_connection_health assertions from test_connections_read.py - document the breaking removals in CHANGELOG.md and update README
| health = connections_api.check_connection_health(connection_id) | ||
| assert health.connection_id == connection_id | ||
| assert health.healthy, f"seeded connection unhealthy: {health.error}" | ||
|
|
||
| # purge_connection_cache returns None on success. | ||
| connections_api.purge_connection_cache(connection_id) |
There was a problem hiding this comment.
super nit: (not blocking) the module docstring on line 3 still lists "health check" among the operations. The check_connection_health call is gone from this test. Drop that phrase so the docstring matches the test body.
There was a problem hiding this comment.
Prior blocking issue is resolved. CHANGELOG.md now documents the removed Secrets, Refresh, and Connection Types surface under a ### Removed block with **Breaking:** markers. No remaining references to the removed symbols exist in the repo. One super nit inline.
1 participant
Auto-generated from the updated HotData OpenAPI spec.
Source: https://github.com/hotdata-dev/www.hotdata.dev/pull/391