Skip to content

Tackle issues#264

Closed
domino-blake wants to merge 20 commits into
masterfrom
tackle-issues
Closed

Tackle issues#264
domino-blake wants to merge 20 commits into
masterfrom
tackle-issues

Conversation

@domino-blake
Copy link
Copy Markdown
Contributor

PR: SDK improvements, type safety, CI, and issue fixes

Summary

This branch consolidates a broad set of improvements across the SDK: code quality
(mypy, lint, formatting), CI/CD infrastructure, deprecation warnings for renamed
parameters, new user-facing features, a bug fix, and documentation updates.


What changed

CI / tooling

  • Added GitHub Actions workflow (.github/workflows/ci.yml) with lint, typecheck,
    and test jobs that gate PRs to master
  • Rewrote tox.ini with Python 3.10/3.11/3.12 envs and a nosdk env for data-source
    tests without the optional SDK
  • Added coverage config to pytest.ini
  • Added scripts/check_snake_case.py and corresponding pre-commit hook to catch
    camelCase public identifiers
  • Updated all pre-commit hook versions to latest

Type safety

  • Resolved all 38 pre-existing mypy errors across the codebase (domino/domino.py,
    domino/datasets.py, domino/_custom_metrics.py, domino/airflow/_operator.py,
    domino/agents/ — no new errors introduced)

Deprecation warnings

  • Renamed all public camelCase parameters to snake_case with backwards-compatible
    shims that emit DeprecationWarning at call time:
    • isDirectis_direct, commitIdcommit_id, publishApiEndpoint
      publish_api_endpoint (runs API)
    • unpublishRunningAppsunpublish_running_apps, hardwareTierId
      hardware_tier_id, environmentIdenvironment_id, externalVolumeMountIds
      external_volume_mount_ids, appIdapp_id (app API)
    • includeSetupLoginclude_setup_log (runs stdout)
  • Added tests/test_deprecations.py with full coverage of all shims

New features

Bug fix

  • _validate_hardware_tier_id dict input — some call paths (e.g.
    compute_cluster_properties["workerHardwareTierId"]) pass a dict like
    {"value": "small-k8s"} instead of a plain string, causing HardwareTierNotFoundException
    even when the tier exists. The method now extracts the string value from the dict
    before comparing. Closes BUG: in _validate_hardware_tier_id #174.

Documentation

  • Added a runs_start vs job_start comparison table to both README.md and
    README.adoc explaining API version, feature support, and when to use each. Closes Document The Differnce Between domino.runs_start() and domino.job_start() #122.
  • Updated README.md and README.adoc with full documentation for all new methods
    and properties: files_download, app_get_status, app_id, and the branch
    param on job_start.
  • Updated CHANGELOG.md.

Issues closed

Issue Title
#24 Download file without blob ID
#31 Blob get by file name
#122 Document runs_start vs job_start
#127 Make _app_id a public property
#128 Make __app_get_status public
#174 _validate_hardware_tier_id receives a dict
#231 job_start branch support

Test plan

  • [*] tox passes across py310, py311, py312 (188 passed, 55 skipped)
  • [*] All lint checks pass: black, isort, flake8, mypy, check_snake_case
  • [*] GitHub Actions CI passes on the PR
  • [*] New tests added for every feature and bug fix:
    • tests/test_jobs.py — branch param, _validate_hardware_tier_id dict input
    • tests/test_app.pyapp_get_status (status present and missing)
    • tests/test_apps.pyapp_id property (app exists, no app)
    • tests/test_files.pyfiles_download (explicit commit, latest commit default)
    • tests/test_deprecations.py — all camelCase shims

  - Add .github/workflows/ci.yml with lint, typecheck, and test jobs
    gating every PR and push to master
  - Rewrite tox.ini to run full suite across Python 3.10/3.11/3.12
  - Add coverage config to pytest.ini
  - Update CHANGELOG
…both theheading and parameter list

- Documented all 7 parameters (including the 4 that were already there but undocumented: environment_id, external_volume_mount_ids, commit_id, branch, app_id)
- Added code examples showing the three common new use cases (branch, commit, specific app ID)
- Added a deprecation note pointing callers from the old camelCase names to the new ones
- Updated app_unpublish() to document its app_id parameter (which was also previously undocumented)
- Updated app_publish() to include branch and commit_id when publishing
Copy link
Copy Markdown
Contributor

@ddl-bira-ignacio ddl-bira-ignacio left a comment

Choose a reason for hiding this comment

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

I think we can split this up in a few ways. Maybe like this

  • CI: GitHub Actions, tox, pytest/coverage, pre-commit bumps (config only)
  • Format: black/isort/flake8/ruff mechanical fixes, no behavior change
  • Types: all the mypy fixes only
  • Deprecations: camelCase, snake_case + shims + test_deprecations.py
  • linting: – check_snake_case.py, hooks
  • Bugfixes: also split by file download, jobs, apps
  • Documentation

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