Skip to content

chore(jobs): remove data_refresh_table and data_refresh_connection job types - #179

Open
hotdata-automation[bot] wants to merge 1 commit into
mainfrom
openapi-update-32806824789
Open

chore(jobs): remove data_refresh_table and data_refresh_connection job types#179
hotdata-automation[bot] wants to merge 1 commit into
mainfrom
openapi-update-32806824789

Conversation

@hotdata-automation

Copy link
Copy Markdown
Contributor

Auto-generated from the updated HotData OpenAPI spec.
Source: https://github.com/hotdata-dev/www.hotdata.dev/pull/389

@hotdata-automation
hotdata-automation Bot requested a review from a team as a code owner August 25, 2026 03:53
@hotdata-automation
hotdata-automation Bot requested review from rohan-hotdata and removed request for a team August 25, 2026 03:53
@hotdata-automation
hotdata-automation Bot enabled auto-merge (squash) August 25, 2026 03:53
Comment thread CHANGELOG.md

### Changed

- chore(jobs): remove data_refresh_table and data_refresh_connection job types

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Blocking: the changelog entry describes only the job-type removal, but this PR also deletes the entire SecretsApi, ConnectionTypesApi, and RefreshApi, removes ConnectionsApi.check_connection_health, and drops ~20 public models (CreateSecretRequest, SecretMetadataResponse, RefreshRequest, ConnectionTypeSummary, ConnectionHealthResponse, …). Those are all exported from hotdata/__init__.py today, so this is a hard breaking change for any caller importing them.

Either the removals are unintended (in which case the spec sync pulled in more than the linked upstream PR describes), or they need to be documented. If intended, please:

  • move them under a ### Removed heading with a Breaking: marker, matching the style of the 0.10.0 JWT-exchange entry
  • update the PR title/description, which currently reads as a narrow job-type cleanup

Also note the version is still 0.10.0; a breaking removal set should carry the corresponding bump before release.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Review

Note on context: the full diff was unavailable in the review prompt and gh pr diff refuses this PR (HTTP 406: diff exceeded the maximum number of files (300)); the changed-file list was also truncated (the API returns only the first 100 of 326 paths). I reviewed by reading the post-merge checkout directly — hotdata/models/job_type.py, hotdata/models/job_result.py, hotdata/api/*.py, CHANGELOG.md, README.md, .github/workflows/integration-tests.yml, and the whole tests/integration/ suite — and by grepping the tree for references to every removed symbol. The job-type/job-result changes named in the title are internally consistent; the findings below are about everything else this PR removes.

Blocking Issues

1. tests/integration/conftest.py imports three deleted modules — the entire integration suite fails at collection.

tests/integration/conftest.py:17: from hotdata.api.connection_types_api import ConnectionTypesApi
tests/integration/conftest.py:25: from hotdata.api.refresh_api import RefreshApi
tests/integration/conftest.py:27: from hotdata.api.secrets_api import SecretsApi

All three modules are deleted by this PR and no longer exist in hotdata/api/. .github/workflows/integration-tests.yml:99 runs pytest tests/integration -v, so this is an ImportError during conftest import — every integration test errors out, not just the ones that touch these APIs. The secrets_api / connection_types_api / refresh_api fixtures at lines 155, 170, and 190 need to go with them.

2. tests/integration/test_secrets_crud.py and tests/integration/test_connection_types_read.py target removed APIs.

Both import deleted modules at file scope (test_secrets_crud.py:15,17,18, test_connection_types_read.py:9) and exercise endpoints the SDK no longer exposes. They should be deleted alongside the generated test/test_secret_*.py / test/test_connection_type_*.py files this PR already removes.

Worth checking against the upstream scenario manifest: scenario-parity passed, but that check only flags scenarios missing a test file (integration-tests.yml:49-51) — it cannot catch a test file for a scenario that no longer exists. If secrets_crud and connection_types_read are still in api/test-scenarios.yaml, removing these files will flip that check red, which means the manifest needs the matching update upstream first.

3. tests/integration/test_connections_read.py:31 calls check_connection_health, which this PR removes from ConnectionsApi.

health = connections_api.check_connection_health(connection_id)

ConnectionsApi no longer defines it (the -267 lines in hotdata/api/connections_api.py, plus the deletion of ConnectionHealthResponse). A grep for check_connection_health across the tree now matches this line and nothing else. Even once finding 1 is fixed, this test fails with AttributeError.

4. Undocumented breaking removals — see the inline comment on CHANGELOG.md:12.

The changelog and PR title cover only the job-type removal, while the PR also deletes SecretsApi, ConnectionTypesApi, RefreshApi, ConnectionsApi.check_connection_health, and ~20 exported models.

Action Required

  • Confirm the Secrets / ConnectionTypes / Refresh / connection-health removals are actually intended. The linked upstream PR is titled as a job-type cleanup, and a spec sync that silently drops four API surfaces is worth double-checking before it lands.
  • If intended: drop the three dead fixtures and their imports from tests/integration/conftest.py, delete test_secrets_crud.py and test_connection_types_read.py, remove the check_connection_health assertions from test_connections_read.py, and coordinate the scenario-manifest update upstream.
  • Document the removals in CHANGELOG.md under a Removed heading with a Breaking: marker, and update the PR title/description to reflect the real scope.

Nits

README.md:3 still advertises secrets in the list of covered API surfaces (… SQL queries, results, secrets, uploads, indexes, jobs …). Drop it if the Secrets API removal is intended. (not blocking)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants