rename camelCase params to snake_case with deprecation shims#267
Conversation
domino-blake
changed the title
|
Once #265 merges, need to change this to target master |
|
issue: The table says endpoint_publish renames modelId/modelVersionId, but the actual diff renames commitId to commit_id. Looks like the table just needs a small fix. |
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
| is_direct = kwargs.pop("isDirect") |
There was a problem hiding this comment.
issue: If someone passes both is_direct=... and isDirect=... in the same call, the old name silently wins here because kwargs gets popped after binding. Might be worth raising a ValueError when both are provided so nobody gets surprised.
|
|
||
| - name: snake_case | ||
| run: | | ||
| find domino -name "*.py" \ |
There was a problem hiding this comment.
suggestion: The find/grep combo here seems different than what the pre-commit hook does
There was a problem hiding this comment.
fixed by moving the filtering rules into check_snake_case.py itself, so both CI and pre-commit invoke the same 9987404
domino-blake
force-pushed
the
blake/pr-renames
branch
from
f996a64 to
3402e8a
Compare
| return run_response | ||
|
|
||
| def run_stop(self, runId, saveChanges=True): | ||
| def run_stop(self, run_id=_UNSET, save_changes=_UNSET, **kwargs): |
There was a problem hiding this comment.
suggestion: run_stop with no args now silently passes None as job_id instead of raising, as it would before. It might be worth putting code that checks if there's a valid run id passed in
The conflict resolution for commit b45ea77 left an extra blank line between the _resolve_renamed_kwarg helper and the Domino class. Black removed it. No semantic change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
domino-blake
force-pushed
the
blake/pr-renames
branch
from
bd1785b to
9a24604
Compare
2 participants
What does this PR do?
Renames 18 public API parameters from camelCase to snake_case to comply with PEP 8.
Old names continue to work and emit a
DeprecationWarningpointing callers to thenew name — no hard breaking changes in this PR.
Renamed parameters
runs_start/runs_start_blockingisDirectis_directruns_start/runs_start_blockingcommitIdcommit_idruns_start/runs_start_blockingpublishApiEndpointpublish_api_endpointrun_stopsaveChangessave_changesruns_statusrunIdrun_idget_run_logincludeSetupLoginclude_setup_logruns_stdoutrunIdrun_idfiles_listcommitIdcommit_idendpoint_publishmodelId/modelVersionIdmodel_id/model_version_idapp_publishunpublishRunningApps/hardwareTierId/environmentId/externalVolumeMountIds/commitId/appIdTests
tests/test_deprecations.pywith full coverage: every old name raisesDeprecationWarning, every new name is accepted silently.Note for reviewers
These are technically breaking in a future major version (old names will eventually
be removed). The deprecation shims give callers time to migrate. Happy to discuss
the timeline for removal if needed.
Testing
18 new tests in
tests/test_deprecations.pyAll existing tests continue to pass (they use the old names, which still work)
Unit tests passing
Deprecation warnings added for all renamed params
CHANGELOG updated
README updated