feat(cli+sdk): Tower Storage from the CLI — credential vending & connection snippets#309
Open
konstantinoscs wants to merge 1 commit into
Conversation
Contributor
|
This PR is targeting If this is a regular feature/fix PR, please change the base branch to Current base: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-binaries.yml:
- Around line 99-104: The Intel macOS wheel job builds and uploads the wheel
without validating it, so add the same smoke-test step used in the other wheel
workflows after the Build wheel step in this job. Use the produced artifact from
the maturin-action output to install the wheel with the matrix Python
interpreter, then run the wheel’s entrypoint/import check so a bad tag or broken
install fails before Upload wheel. Reference the existing Build wheel and Upload
wheel steps in build-binaries.yml and mirror the pattern from the other wheel
jobs.
- Around line 204-206: The checkout step for the containerized build jobs still
leaves persisted credentials available in the workspace, unlike the existing
linux-cross setup. Update the actions/checkout usage in the musllinux and
musllinux-cross job paths to disable credential persistence by adding the same
persist-credentials setting used elsewhere, so the checked-out token is not
exposed to addnab/docker-run-action or uraimo/run-on-arch-action.
- Line 202: The wheel artifact names in the build matrix are colliding across
Python versions, so each upload should be uniquely named per target and
python-version. Update the artifact name used by the upload step in the workflow
to include the matrix python-version suffix, using the existing matrix fields
such as matrix.python-version so Linux, musllinux, and musllinux-cross jobs
preserve distinct wheel sets instead of overwriting each other.
In `@crates/tower-cmd/src/api.rs`:
- Around line 381-405: `vend_catalog_credentials` is passing a sensitive
bearer-token response into `unwrap_api_response`, which can expose
`credentials.oauth_token` in debug logs. Update `vend_catalog_credentials` (and,
if needed, the `unwrap_api_response` call path) to redact or avoid logging the
raw response body for this API before returning the
`VendCatalogCredentialsResponse`, so debug output never prints the token unless
explicitly allowed by the command.
In `@crates/tower-cmd/src/catalogs.rs`:
- Around line 241-247: The token export helper currently hardcodes a POSIX shell
snippet in token_export_command, which won’t work for Windows binaries. Update
token_export_command to detect the target platform and emit a platform-specific
instruction: keep the existing export-based flow for Unix-like shells, but on
Windows return a PowerShell-friendly or manual set/copy command instead of
relying on export, shell quoting, and python3. Use the existing
token_export_command symbol as the place to branch the output.
In `@src/tower/_context.py`:
- Around line 65-83: Normalize empty auth environment values in the
`_context.py` initialization path by treating blank `TOWER_API_KEY` and
`TOWER_JWT` the same as unset before the `_read_session()` fallback. Update the
`tower_api_key`/`tower_jwt` handling in the context-loading logic so `""` does
not count as present, allowing `tower login` session values to be used when env
vars are exported empty. Keep the existing `tower_url` and `tower_run_id` flow
unchanged.
In `@src/tower/_tables.py`:
- Around line 87-97: The current read-write fast path in
_ensure_read_write_table prevents re-entering get_tower_catalog() once
_catalog_mode is already "read-write", which skips the _storage expires_at
refresh for long-lived handles. Update the write/drop/create flow so existing
read-write catalogs still go back through get_tower_catalog() when needed, while
letting the credential cache decide whether to vend new credentials. Keep the
fix centered on _ensure_read_write_table and the related read-write call sites
so refreshed storage credentials are picked up before subsequent operations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec0f94ad-4db6-4f0e-bd60-f3b163603e8e
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (247)
.github/workflows/build-binaries.ymlcrates/tower-api/README.mdcrates/tower-api/src/apis/configuration.rscrates/tower-api/src/apis/default_api.rscrates/tower-api/src/apis/feature_flags_api.rscrates/tower-api/src/models/account.rscrates/tower-api/src/models/acknowledge_alert_response.rscrates/tower-api/src/models/acknowledge_all_alerts_response.rscrates/tower-api/src/models/alert.rscrates/tower-api/src/models/api_key.rscrates/tower-api/src/models/api_key_owner.rscrates/tower-api/src/models/app.rscrates/tower-api/src/models/app_statistics.rscrates/tower-api/src/models/app_summary.rscrates/tower-api/src/models/app_version.rscrates/tower-api/src/models/authentication_context.rscrates/tower-api/src/models/batch_schedule_params.rscrates/tower-api/src/models/batch_schedule_response.rscrates/tower-api/src/models/cancel_run_response.rscrates/tower-api/src/models/catalog.rscrates/tower-api/src/models/catalog_credentials.rscrates/tower-api/src/models/catalog_property.rscrates/tower-api/src/models/claim_device_login_ticket_params.rscrates/tower-api/src/models/claim_device_login_ticket_response.rscrates/tower-api/src/models/create_account_params.rscrates/tower-api/src/models/create_account_params_flags_struct.rscrates/tower-api/src/models/create_account_response.rscrates/tower-api/src/models/create_api_key_params.rscrates/tower-api/src/models/create_api_key_response.rscrates/tower-api/src/models/create_app_params.rscrates/tower-api/src/models/create_app_response.rscrates/tower-api/src/models/create_catalog_params.rscrates/tower-api/src/models/create_catalog_response.rscrates/tower-api/src/models/create_device_login_ticket_response.rscrates/tower-api/src/models/create_environment_params.rscrates/tower-api/src/models/create_environment_response.rscrates/tower-api/src/models/create_guest_params.rscrates/tower-api/src/models/create_guest_response.rscrates/tower-api/src/models/create_sandbox_secrets_params.rscrates/tower-api/src/models/create_sandbox_secrets_response.rscrates/tower-api/src/models/create_schedule_params.rscrates/tower-api/src/models/create_schedule_response.rscrates/tower-api/src/models/create_secret_params.rscrates/tower-api/src/models/create_secret_response.rscrates/tower-api/src/models/create_service_account_api_key_params.rscrates/tower-api/src/models/create_service_account_api_key_response.rscrates/tower-api/src/models/create_service_account_params.rscrates/tower-api/src/models/create_service_account_response.rscrates/tower-api/src/models/create_session_params.rscrates/tower-api/src/models/create_session_response.rscrates/tower-api/src/models/create_team_params.rscrates/tower-api/src/models/create_team_response.rscrates/tower-api/src/models/create_webhook_params.rscrates/tower-api/src/models/create_webhook_response.rscrates/tower-api/src/models/delete_api_key_params.rscrates/tower-api/src/models/delete_api_key_response.rscrates/tower-api/src/models/delete_app_response.rscrates/tower-api/src/models/delete_catalog_response.rscrates/tower-api/src/models/delete_environment_response.rscrates/tower-api/src/models/delete_guest_output_body.rscrates/tower-api/src/models/delete_schedule_params.rscrates/tower-api/src/models/delete_schedule_response.rscrates/tower-api/src/models/delete_secret_response.rscrates/tower-api/src/models/delete_service_account_api_key_params.rscrates/tower-api/src/models/delete_session_params.rscrates/tower-api/src/models/delete_session_response.rscrates/tower-api/src/models/delete_team_invitation_params.rscrates/tower-api/src/models/delete_team_invitation_response.rscrates/tower-api/src/models/delete_team_params.rscrates/tower-api/src/models/delete_team_response.rscrates/tower-api/src/models/delete_webhook_response.rscrates/tower-api/src/models/deploy_app_request.rscrates/tower-api/src/models/deploy_app_response.rscrates/tower-api/src/models/describe_account_body.rscrates/tower-api/src/models/describe_app_response.rscrates/tower-api/src/models/describe_app_version_response.rscrates/tower-api/src/models/describe_authentication_context_body.rscrates/tower-api/src/models/describe_catalog_response.rscrates/tower-api/src/models/describe_device_login_session_response.rscrates/tower-api/src/models/describe_email_preferences_body.rscrates/tower-api/src/models/describe_environment_response.rscrates/tower-api/src/models/describe_plan_response.rscrates/tower-api/src/models/describe_run_graph_response.rscrates/tower-api/src/models/describe_run_links.rscrates/tower-api/src/models/describe_run_logs_response.rscrates/tower-api/src/models/describe_run_response.rscrates/tower-api/src/models/describe_secrets_key_response.rscrates/tower-api/src/models/describe_service_account_response.rscrates/tower-api/src/models/describe_session_response.rscrates/tower-api/src/models/describe_team_response.rscrates/tower-api/src/models/describe_webhook_response.rscrates/tower-api/src/models/describe_whoami_response.rscrates/tower-api/src/models/email_subscriptions.rscrates/tower-api/src/models/encrypted_catalog_property.rscrates/tower-api/src/models/environment.rscrates/tower-api/src/models/error_detail.rscrates/tower-api/src/models/error_model.rscrates/tower-api/src/models/event_alert.rscrates/tower-api/src/models/event_error.rscrates/tower-api/src/models/event_log.rscrates/tower-api/src/models/event_shouldertap.rscrates/tower-api/src/models/event_warning.rscrates/tower-api/src/models/export_catalogs_params.rscrates/tower-api/src/models/export_catalogs_response.rscrates/tower-api/src/models/export_secrets_params.rscrates/tower-api/src/models/export_secrets_response.rscrates/tower-api/src/models/exported_catalog.rscrates/tower-api/src/models/exported_catalog_property.rscrates/tower-api/src/models/exported_secret.rscrates/tower-api/src/models/feature.rscrates/tower-api/src/models/featurebase_identity.rscrates/tower-api/src/models/generate_app_statistics_response.rscrates/tower-api/src/models/generate_organization_usage_time_series_response.rscrates/tower-api/src/models/generate_run_statistics_response.rscrates/tower-api/src/models/generate_runner_credentials_response.rscrates/tower-api/src/models/get_feature_flag_response_body.rscrates/tower-api/src/models/guest.rscrates/tower-api/src/models/invite_team_member_params.rscrates/tower-api/src/models/invite_team_member_response.rscrates/tower-api/src/models/leave_team_response.rscrates/tower-api/src/models/list_alerts_response.rscrates/tower-api/src/models/list_api_keys_response.rscrates/tower-api/src/models/list_app_environments_response.rscrates/tower-api/src/models/list_app_versions_response.rscrates/tower-api/src/models/list_apps_response.rscrates/tower-api/src/models/list_catalogs_response.rscrates/tower-api/src/models/list_environments_response.rscrates/tower-api/src/models/list_guests_response.rscrates/tower-api/src/models/list_my_team_invitations_response.rscrates/tower-api/src/models/list_runners_response.rscrates/tower-api/src/models/list_runs_response.rscrates/tower-api/src/models/list_schedules_response.rscrates/tower-api/src/models/list_secret_environments_response.rscrates/tower-api/src/models/list_secrets_response.rscrates/tower-api/src/models/list_service_account_api_keys_response.rscrates/tower-api/src/models/list_service_accounts_response.rscrates/tower-api/src/models/list_team_invitations_response.rscrates/tower-api/src/models/list_team_members_response.rscrates/tower-api/src/models/list_teams_response.rscrates/tower-api/src/models/list_webhooks_response.rscrates/tower-api/src/models/organization.rscrates/tower-api/src/models/organization_usage.rscrates/tower-api/src/models/pagination.rscrates/tower-api/src/models/parameter.rscrates/tower-api/src/models/plan.rscrates/tower-api/src/models/refresh_session_params.rscrates/tower-api/src/models/refresh_session_response.rscrates/tower-api/src/models/regenerate_guest_login_url_params.rscrates/tower-api/src/models/regenerate_guest_login_url_response.rscrates/tower-api/src/models/remove_team_member_params.rscrates/tower-api/src/models/remove_team_member_response.rscrates/tower-api/src/models/resend_team_invitation_params.rscrates/tower-api/src/models/resend_team_invitation_response.rscrates/tower-api/src/models/run.rscrates/tower-api/src/models/run_app_initiator_data.rscrates/tower-api/src/models/run_app_params.rscrates/tower-api/src/models/run_app_response.rscrates/tower-api/src/models/run_attempt.rscrates/tower-api/src/models/run_creator.rscrates/tower-api/src/models/run_failure_alert.rscrates/tower-api/src/models/run_graph_node.rscrates/tower-api/src/models/run_graph_run_id.rscrates/tower-api/src/models/run_initiator.rscrates/tower-api/src/models/run_initiator_details.rscrates/tower-api/src/models/run_log_line.rscrates/tower-api/src/models/run_parameter.rscrates/tower-api/src/models/run_results.rscrates/tower-api/src/models/run_retry_policy.rscrates/tower-api/src/models/run_run_initiator_details.rscrates/tower-api/src/models/run_statistics.rscrates/tower-api/src/models/run_timeseries_point.rscrates/tower-api/src/models/runner.rscrates/tower-api/src/models/runner_credentials.rscrates/tower-api/src/models/schedule.rscrates/tower-api/src/models/schedule_owner.rscrates/tower-api/src/models/schedule_run_initiator_details.rscrates/tower-api/src/models/search_runs_response.rscrates/tower-api/src/models/secret.rscrates/tower-api/src/models/server_sent_events_inner.rscrates/tower-api/src/models/server_sent_events_inner_1.rscrates/tower-api/src/models/server_sent_events_inner_2.rscrates/tower-api/src/models/service_account.rscrates/tower-api/src/models/service_account_creator.rscrates/tower-api/src/models/session.rscrates/tower-api/src/models/shoulder_tap.rscrates/tower-api/src/models/sse_warning.rscrates/tower-api/src/models/statistics_settings.rscrates/tower-api/src/models/team.rscrates/tower-api/src/models/team_invitation.rscrates/tower-api/src/models/team_membership.rscrates/tower-api/src/models/test_webhook_response.rscrates/tower-api/src/models/token.rscrates/tower-api/src/models/update_account_params.rscrates/tower-api/src/models/update_account_response.rscrates/tower-api/src/models/update_app_environment_params.rscrates/tower-api/src/models/update_app_environment_response.rscrates/tower-api/src/models/update_app_params.rscrates/tower-api/src/models/update_app_response.rscrates/tower-api/src/models/update_catalog_params.rscrates/tower-api/src/models/update_catalog_response.rscrates/tower-api/src/models/update_email_preferences_body.rscrates/tower-api/src/models/update_environment_params.rscrates/tower-api/src/models/update_environment_response.rscrates/tower-api/src/models/update_my_team_invitation_params.rscrates/tower-api/src/models/update_my_team_invitation_response.rscrates/tower-api/src/models/update_organization_params.rscrates/tower-api/src/models/update_organization_response.rscrates/tower-api/src/models/update_plan_params.rscrates/tower-api/src/models/update_plan_response.rscrates/tower-api/src/models/update_schedule_params.rscrates/tower-api/src/models/update_schedule_response.rscrates/tower-api/src/models/update_secret_params.rscrates/tower-api/src/models/update_secret_response.rscrates/tower-api/src/models/update_service_account_params.rscrates/tower-api/src/models/update_service_account_response.rscrates/tower-api/src/models/update_team_member_params.rscrates/tower-api/src/models/update_team_member_response.rscrates/tower-api/src/models/update_team_params.rscrates/tower-api/src/models/update_team_response.rscrates/tower-api/src/models/update_user_params.rscrates/tower-api/src/models/update_user_response.rscrates/tower-api/src/models/update_webhook_params.rscrates/tower-api/src/models/update_webhook_response.rscrates/tower-api/src/models/usage_limit.rscrates/tower-api/src/models/usage_metric_time_series_point.rscrates/tower-api/src/models/user.rscrates/tower-api/src/models/vend_catalog_credentials_body.rscrates/tower-api/src/models/vend_catalog_credentials_response.rscrates/tower-api/src/models/webhook.rscrates/tower-cmd/src/api.rscrates/tower-cmd/src/catalogs.rscrates/tower-cmd/src/lib.rscrates/tower-cmd/src/mcp.rscrates/tower-cmd/src/templates/dbeaver.txt.tmplcrates/tower-cmd/src/templates/dbt.yml.tmplcrates/tower-cmd/src/templates/duckdb.sql.tmplcrates/tower-cmd/src/templates/pyiceberg.py.tmplcrates/tower-cmd/src/templates/spark.py.tmplpyproject.tomlsrc/tower/_context.pysrc/tower/_storage.pysrc/tower/_tables.pysrc/tower/tower_api_client/api/default/generate_organization_usage_time_series.pysrc/tower/tower_api_client/api/default/vend_catalog_credentials.pysrc/tower/tower_api_client/models/vend_catalog_credentials_body.pytests/tower/test_storage.pytests/tower/test_tables.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build-binaries.yml:
- Around line 99-104: The Intel macOS wheel job builds and uploads the wheel
without validating it, so add the same smoke-test step used in the other wheel
workflows after the Build wheel step in this job. Use the produced artifact from
the maturin-action output to install the wheel with the matrix Python
interpreter, then run the wheel’s entrypoint/import check so a bad tag or broken
install fails before Upload wheel. Reference the existing Build wheel and Upload
wheel steps in build-binaries.yml and mirror the pattern from the other wheel
jobs.
- Around line 204-206: The checkout step for the containerized build jobs still
leaves persisted credentials available in the workspace, unlike the existing
linux-cross setup. Update the actions/checkout usage in the musllinux and
musllinux-cross job paths to disable credential persistence by adding the same
persist-credentials setting used elsewhere, so the checked-out token is not
exposed to addnab/docker-run-action or uraimo/run-on-arch-action.
- Line 202: The wheel artifact names in the build matrix are colliding across
Python versions, so each upload should be uniquely named per target and
python-version. Update the artifact name used by the upload step in the workflow
to include the matrix python-version suffix, using the existing matrix fields
such as matrix.python-version so Linux, musllinux, and musllinux-cross jobs
preserve distinct wheel sets instead of overwriting each other.
In `@crates/tower-cmd/src/api.rs`:
- Around line 381-405: `vend_catalog_credentials` is passing a sensitive
bearer-token response into `unwrap_api_response`, which can expose
`credentials.oauth_token` in debug logs. Update `vend_catalog_credentials` (and,
if needed, the `unwrap_api_response` call path) to redact or avoid logging the
raw response body for this API before returning the
`VendCatalogCredentialsResponse`, so debug output never prints the token unless
explicitly allowed by the command.
In `@crates/tower-cmd/src/catalogs.rs`:
- Around line 241-247: The token export helper currently hardcodes a POSIX shell
snippet in token_export_command, which won’t work for Windows binaries. Update
token_export_command to detect the target platform and emit a platform-specific
instruction: keep the existing export-based flow for Unix-like shells, but on
Windows return a PowerShell-friendly or manual set/copy command instead of
relying on export, shell quoting, and python3. Use the existing
token_export_command symbol as the place to branch the output.
In `@src/tower/_context.py`:
- Around line 65-83: Normalize empty auth environment values in the
`_context.py` initialization path by treating blank `TOWER_API_KEY` and
`TOWER_JWT` the same as unset before the `_read_session()` fallback. Update the
`tower_api_key`/`tower_jwt` handling in the context-loading logic so `""` does
not count as present, allowing `tower login` session values to be used when env
vars are exported empty. Keep the existing `tower_url` and `tower_run_id` flow
unchanged.
In `@src/tower/_tables.py`:
- Around line 87-97: The current read-write fast path in
_ensure_read_write_table prevents re-entering get_tower_catalog() once
_catalog_mode is already "read-write", which skips the _storage expires_at
refresh for long-lived handles. Update the write/drop/create flow so existing
read-write catalogs still go back through get_tower_catalog() when needed, while
letting the credential cache decide whether to vend new credentials. Keep the
fix centered on _ensure_read_write_table and the related read-write call sites
so refreshed storage credentials are picked up before subsequent operations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec0f94ad-4db6-4f0e-bd60-f3b163603e8e
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (247)
.github/workflows/build-binaries.ymlcrates/tower-api/README.mdcrates/tower-api/src/apis/configuration.rscrates/tower-api/src/apis/default_api.rscrates/tower-api/src/apis/feature_flags_api.rscrates/tower-api/src/models/account.rscrates/tower-api/src/models/acknowledge_alert_response.rscrates/tower-api/src/models/acknowledge_all_alerts_response.rscrates/tower-api/src/models/alert.rscrates/tower-api/src/models/api_key.rscrates/tower-api/src/models/api_key_owner.rscrates/tower-api/src/models/app.rscrates/tower-api/src/models/app_statistics.rscrates/tower-api/src/models/app_summary.rscrates/tower-api/src/models/app_version.rscrates/tower-api/src/models/authentication_context.rscrates/tower-api/src/models/batch_schedule_params.rscrates/tower-api/src/models/batch_schedule_response.rscrates/tower-api/src/models/cancel_run_response.rscrates/tower-api/src/models/catalog.rscrates/tower-api/src/models/catalog_credentials.rscrates/tower-api/src/models/catalog_property.rscrates/tower-api/src/models/claim_device_login_ticket_params.rscrates/tower-api/src/models/claim_device_login_ticket_response.rscrates/tower-api/src/models/create_account_params.rscrates/tower-api/src/models/create_account_params_flags_struct.rscrates/tower-api/src/models/create_account_response.rscrates/tower-api/src/models/create_api_key_params.rscrates/tower-api/src/models/create_api_key_response.rscrates/tower-api/src/models/create_app_params.rscrates/tower-api/src/models/create_app_response.rscrates/tower-api/src/models/create_catalog_params.rscrates/tower-api/src/models/create_catalog_response.rscrates/tower-api/src/models/create_device_login_ticket_response.rscrates/tower-api/src/models/create_environment_params.rscrates/tower-api/src/models/create_environment_response.rscrates/tower-api/src/models/create_guest_params.rscrates/tower-api/src/models/create_guest_response.rscrates/tower-api/src/models/create_sandbox_secrets_params.rscrates/tower-api/src/models/create_sandbox_secrets_response.rscrates/tower-api/src/models/create_schedule_params.rscrates/tower-api/src/models/create_schedule_response.rscrates/tower-api/src/models/create_secret_params.rscrates/tower-api/src/models/create_secret_response.rscrates/tower-api/src/models/create_service_account_api_key_params.rscrates/tower-api/src/models/create_service_account_api_key_response.rscrates/tower-api/src/models/create_service_account_params.rscrates/tower-api/src/models/create_service_account_response.rscrates/tower-api/src/models/create_session_params.rscrates/tower-api/src/models/create_session_response.rscrates/tower-api/src/models/create_team_params.rscrates/tower-api/src/models/create_team_response.rscrates/tower-api/src/models/create_webhook_params.rscrates/tower-api/src/models/create_webhook_response.rscrates/tower-api/src/models/delete_api_key_params.rscrates/tower-api/src/models/delete_api_key_response.rscrates/tower-api/src/models/delete_app_response.rscrates/tower-api/src/models/delete_catalog_response.rscrates/tower-api/src/models/delete_environment_response.rscrates/tower-api/src/models/delete_guest_output_body.rscrates/tower-api/src/models/delete_schedule_params.rscrates/tower-api/src/models/delete_schedule_response.rscrates/tower-api/src/models/delete_secret_response.rscrates/tower-api/src/models/delete_service_account_api_key_params.rscrates/tower-api/src/models/delete_session_params.rscrates/tower-api/src/models/delete_session_response.rscrates/tower-api/src/models/delete_team_invitation_params.rscrates/tower-api/src/models/delete_team_invitation_response.rscrates/tower-api/src/models/delete_team_params.rscrates/tower-api/src/models/delete_team_response.rscrates/tower-api/src/models/delete_webhook_response.rscrates/tower-api/src/models/deploy_app_request.rscrates/tower-api/src/models/deploy_app_response.rscrates/tower-api/src/models/describe_account_body.rscrates/tower-api/src/models/describe_app_response.rscrates/tower-api/src/models/describe_app_version_response.rscrates/tower-api/src/models/describe_authentication_context_body.rscrates/tower-api/src/models/describe_catalog_response.rscrates/tower-api/src/models/describe_device_login_session_response.rscrates/tower-api/src/models/describe_email_preferences_body.rscrates/tower-api/src/models/describe_environment_response.rscrates/tower-api/src/models/describe_plan_response.rscrates/tower-api/src/models/describe_run_graph_response.rscrates/tower-api/src/models/describe_run_links.rscrates/tower-api/src/models/describe_run_logs_response.rscrates/tower-api/src/models/describe_run_response.rscrates/tower-api/src/models/describe_secrets_key_response.rscrates/tower-api/src/models/describe_service_account_response.rscrates/tower-api/src/models/describe_session_response.rscrates/tower-api/src/models/describe_team_response.rscrates/tower-api/src/models/describe_webhook_response.rscrates/tower-api/src/models/describe_whoami_response.rscrates/tower-api/src/models/email_subscriptions.rscrates/tower-api/src/models/encrypted_catalog_property.rscrates/tower-api/src/models/environment.rscrates/tower-api/src/models/error_detail.rscrates/tower-api/src/models/error_model.rscrates/tower-api/src/models/event_alert.rscrates/tower-api/src/models/event_error.rscrates/tower-api/src/models/event_log.rscrates/tower-api/src/models/event_shouldertap.rscrates/tower-api/src/models/event_warning.rscrates/tower-api/src/models/export_catalogs_params.rscrates/tower-api/src/models/export_catalogs_response.rscrates/tower-api/src/models/export_secrets_params.rscrates/tower-api/src/models/export_secrets_response.rscrates/tower-api/src/models/exported_catalog.rscrates/tower-api/src/models/exported_catalog_property.rscrates/tower-api/src/models/exported_secret.rscrates/tower-api/src/models/feature.rscrates/tower-api/src/models/featurebase_identity.rscrates/tower-api/src/models/generate_app_statistics_response.rscrates/tower-api/src/models/generate_organization_usage_time_series_response.rscrates/tower-api/src/models/generate_run_statistics_response.rscrates/tower-api/src/models/generate_runner_credentials_response.rscrates/tower-api/src/models/get_feature_flag_response_body.rscrates/tower-api/src/models/guest.rscrates/tower-api/src/models/invite_team_member_params.rscrates/tower-api/src/models/invite_team_member_response.rscrates/tower-api/src/models/leave_team_response.rscrates/tower-api/src/models/list_alerts_response.rscrates/tower-api/src/models/list_api_keys_response.rscrates/tower-api/src/models/list_app_environments_response.rscrates/tower-api/src/models/list_app_versions_response.rscrates/tower-api/src/models/list_apps_response.rscrates/tower-api/src/models/list_catalogs_response.rscrates/tower-api/src/models/list_environments_response.rscrates/tower-api/src/models/list_guests_response.rscrates/tower-api/src/models/list_my_team_invitations_response.rscrates/tower-api/src/models/list_runners_response.rscrates/tower-api/src/models/list_runs_response.rscrates/tower-api/src/models/list_schedules_response.rscrates/tower-api/src/models/list_secret_environments_response.rscrates/tower-api/src/models/list_secrets_response.rscrates/tower-api/src/models/list_service_account_api_keys_response.rscrates/tower-api/src/models/list_service_accounts_response.rscrates/tower-api/src/models/list_team_invitations_response.rscrates/tower-api/src/models/list_team_members_response.rscrates/tower-api/src/models/list_teams_response.rscrates/tower-api/src/models/list_webhooks_response.rscrates/tower-api/src/models/organization.rscrates/tower-api/src/models/organization_usage.rscrates/tower-api/src/models/pagination.rscrates/tower-api/src/models/parameter.rscrates/tower-api/src/models/plan.rscrates/tower-api/src/models/refresh_session_params.rscrates/tower-api/src/models/refresh_session_response.rscrates/tower-api/src/models/regenerate_guest_login_url_params.rscrates/tower-api/src/models/regenerate_guest_login_url_response.rscrates/tower-api/src/models/remove_team_member_params.rscrates/tower-api/src/models/remove_team_member_response.rscrates/tower-api/src/models/resend_team_invitation_params.rscrates/tower-api/src/models/resend_team_invitation_response.rscrates/tower-api/src/models/run.rscrates/tower-api/src/models/run_app_initiator_data.rscrates/tower-api/src/models/run_app_params.rscrates/tower-api/src/models/run_app_response.rscrates/tower-api/src/models/run_attempt.rscrates/tower-api/src/models/run_creator.rscrates/tower-api/src/models/run_failure_alert.rscrates/tower-api/src/models/run_graph_node.rscrates/tower-api/src/models/run_graph_run_id.rscrates/tower-api/src/models/run_initiator.rscrates/tower-api/src/models/run_initiator_details.rscrates/tower-api/src/models/run_log_line.rscrates/tower-api/src/models/run_parameter.rscrates/tower-api/src/models/run_results.rscrates/tower-api/src/models/run_retry_policy.rscrates/tower-api/src/models/run_run_initiator_details.rscrates/tower-api/src/models/run_statistics.rscrates/tower-api/src/models/run_timeseries_point.rscrates/tower-api/src/models/runner.rscrates/tower-api/src/models/runner_credentials.rscrates/tower-api/src/models/schedule.rscrates/tower-api/src/models/schedule_owner.rscrates/tower-api/src/models/schedule_run_initiator_details.rscrates/tower-api/src/models/search_runs_response.rscrates/tower-api/src/models/secret.rscrates/tower-api/src/models/server_sent_events_inner.rscrates/tower-api/src/models/server_sent_events_inner_1.rscrates/tower-api/src/models/server_sent_events_inner_2.rscrates/tower-api/src/models/service_account.rscrates/tower-api/src/models/service_account_creator.rscrates/tower-api/src/models/session.rscrates/tower-api/src/models/shoulder_tap.rscrates/tower-api/src/models/sse_warning.rscrates/tower-api/src/models/statistics_settings.rscrates/tower-api/src/models/team.rscrates/tower-api/src/models/team_invitation.rscrates/tower-api/src/models/team_membership.rscrates/tower-api/src/models/test_webhook_response.rscrates/tower-api/src/models/token.rscrates/tower-api/src/models/update_account_params.rscrates/tower-api/src/models/update_account_response.rscrates/tower-api/src/models/update_app_environment_params.rscrates/tower-api/src/models/update_app_environment_response.rscrates/tower-api/src/models/update_app_params.rscrates/tower-api/src/models/update_app_response.rscrates/tower-api/src/models/update_catalog_params.rscrates/tower-api/src/models/update_catalog_response.rscrates/tower-api/src/models/update_email_preferences_body.rscrates/tower-api/src/models/update_environment_params.rscrates/tower-api/src/models/update_environment_response.rscrates/tower-api/src/models/update_my_team_invitation_params.rscrates/tower-api/src/models/update_my_team_invitation_response.rscrates/tower-api/src/models/update_organization_params.rscrates/tower-api/src/models/update_organization_response.rscrates/tower-api/src/models/update_plan_params.rscrates/tower-api/src/models/update_plan_response.rscrates/tower-api/src/models/update_schedule_params.rscrates/tower-api/src/models/update_schedule_response.rscrates/tower-api/src/models/update_secret_params.rscrates/tower-api/src/models/update_secret_response.rscrates/tower-api/src/models/update_service_account_params.rscrates/tower-api/src/models/update_service_account_response.rscrates/tower-api/src/models/update_team_member_params.rscrates/tower-api/src/models/update_team_member_response.rscrates/tower-api/src/models/update_team_params.rscrates/tower-api/src/models/update_team_response.rscrates/tower-api/src/models/update_user_params.rscrates/tower-api/src/models/update_user_response.rscrates/tower-api/src/models/update_webhook_params.rscrates/tower-api/src/models/update_webhook_response.rscrates/tower-api/src/models/usage_limit.rscrates/tower-api/src/models/usage_metric_time_series_point.rscrates/tower-api/src/models/user.rscrates/tower-api/src/models/vend_catalog_credentials_body.rscrates/tower-api/src/models/vend_catalog_credentials_response.rscrates/tower-api/src/models/webhook.rscrates/tower-cmd/src/api.rscrates/tower-cmd/src/catalogs.rscrates/tower-cmd/src/lib.rscrates/tower-cmd/src/mcp.rscrates/tower-cmd/src/templates/dbeaver.txt.tmplcrates/tower-cmd/src/templates/dbt.yml.tmplcrates/tower-cmd/src/templates/duckdb.sql.tmplcrates/tower-cmd/src/templates/pyiceberg.py.tmplcrates/tower-cmd/src/templates/spark.py.tmplpyproject.tomlsrc/tower/_context.pysrc/tower/_storage.pysrc/tower/_tables.pysrc/tower/tower_api_client/api/default/generate_organization_usage_time_series.pysrc/tower/tower_api_client/api/default/vend_catalog_credentials.pysrc/tower/tower_api_client/models/vend_catalog_credentials_body.pytests/tower/test_storage.pytests/tower/test_tables.py
🛑 Comments failed to post (7)
.github/workflows/build-binaries.yml (3)
99-104: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Add the same wheel smoke test on Intel macOS.
This leg now varies the interpreter with
-i python${{ matrix.python-version }}, but unlike the other wheel jobs it never installs the produced wheel. A bad wheel tag or broken entrypoint on macOS x86_64 would still upload successfully.Suggested change
- name: Build wheel uses: PyO3/maturin-action@v1.50.1 with: target: x86_64 args: --release --locked --out dist -i python${{ matrix.python-version }} + - name: Test wheel + run: | + pip install dist/*.whl --force-reinstall + ${{ env.EXECUTABLE_NAME }} --help + python -m ${{ env.MODULE_NAME }} --help - name: Upload wheel📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- name: Build wheel uses: PyO3/maturin-action@v1.50.1 with: target: x86_64 args: --release --locked --out dist -i python${{ matrix.python-version }} - name: Test wheel run: | pip install dist/*.whl --force-reinstall ${{ env.EXECUTABLE_NAME }} --help python -m ${{ env.MODULE_NAME }} --help - name: Upload wheel🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-binaries.yml around lines 99 - 104, The Intel macOS wheel job builds and uploads the wheel without validating it, so add the same smoke-test step used in the other wheel workflows after the Build wheel step in this job. Use the produced artifact from the maturin-action output to install the wheel with the matrix Python interpreter, then run the wheel’s entrypoint/import check so a bad tag or broken install fails before Upload wheel. Reference the existing Build wheel and Upload wheel steps in build-binaries.yml and mirror the pattern from the other wheel jobs.
202-202: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the workflow around the referenced lines nl -ba .github/workflows/build-binaries.yml | sed -n '180,230p;290,325p;335,365p' # Find every upload-artifact name in this workflow rg -n 'upload-artifact|name:\s*wheels-' .github/workflows/build-binaries.ymlRepository: tower/tower-cli
Length of output: 193
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show the relevant sections with line numbers using sed only sed -n '180,230p' .github/workflows/build-binaries.yml | cat -n printf '\n---\n' sed -n '290,325p' .github/workflows/build-binaries.yml | cat -n printf '\n---\n' sed -n '335,365p' .github/workflows/build-binaries.yml | cat -n printf '\n=== matches ===\n' rg -n 'upload-artifact|name:\s*wheels-' .github/workflows/build-binaries.ymlRepository: tower/tower-cli
Length of output: 6104
🏁 Script executed:
#!/bin/bash set -euo pipefail sed -n '220,250p' .github/workflows/build-binaries.yml | cat -n printf '\n---\n' sed -n '325,345p' .github/workflows/build-binaries.yml | cat -n printf '\n---\n' sed -n '370,385p' .github/workflows/build-binaries.yml | cat -nRepository: tower/tower-cli
Length of output: 3087
🏁 Script executed:
#!/bin/bash set -euo pipefail awk 'NR>=175 && NR<=210 {printf "%4d %s\n", NR, $0}' .github/workflows/build-binaries.yml printf '\n---\n' awk 'NR>=300 && NR<=315 {printf "%4d %s\n", NR, $0}' .github/workflows/build-binaries.yml printf '\n---\n' awk 'NR>=340 && NR<=355 {printf "%4d %s\n", NR, $0}' .github/workflows/build-binaries.yml printf '\n---\n' awk 'NR>=372 && NR<=385 {printf "%4d %s\n", NR, $0}' .github/workflows/build-binaries.ymlRepository: tower/tower-cli
Length of output: 3120
🌐 Web query:
actions/upload-artifact v6 same artifact name matrix overwrite behavior duplicate uploads💡 Result:
In actions/upload-artifact v6, the default behavior when attempting to upload an artifact with a name that already exists in the same workflow run is to overwrite the existing artifact [1][2]. This represents a change in behavior from earlier versions (such as v4 and v5), where uploading to the same artifact name across multiple jobs or matrix runs would cause the action to fail with a conflict error [1][3][4]. Key details regarding this behavior include: 1. Overwrite Mechanism: When you use the same artifact name, v6 allows the upload to proceed, effectively replacing the previous artifact with the new one [1][2]. The previous artifact will no longer exist and is assigned a new ID [2]. 2. Matrix Scenarios: Because v6 allows overwriting, matrix jobs that use identical artifact names will cause subsequent jobs to overwrite the artifacts uploaded by previous jobs [1]. To avoid this and ensure all artifacts are preserved, it remains highly recommended to include unique identifiers in the artifact name—such as matrix suffixes (e.g., name: my-artifact-${{ matrix.os }}) [1][3]. 3. Immutable Artifacts: While individual uploads can be overwritten in v6, once an artifact has been successfully created, it is considered immutable within that specific upload action instance [1][2]. Note: If you explicitly want to prevent overwriting, some configurations or specific versions of actions may require you to ensure your naming conventions remain unique, as the overwrite behavior is now the default "last-write-wins" approach [1][2]. Top results: [1][5][2]
Citations:
- 1: thomhurst/TUnit#4071
- 2: https://github.com/actions/upload-artifact/blob/main/README.md
- 3: https://github.com/actions/upload-artifact/blob/c7d193f32edcb7bfad88892161225aeda64e9392/README.md
- 4: https://github.com/actions/upload-artifact?tab=readme-ov-file
- 5: https://github.com/actions/upload-artifact/releases/tag/v6.0.0
Suffix the wheel artifact names with
python-version.
actions/upload-artifact@v6overwrites same-named artifacts, so the Linux, musllinux, and musllinux-cross matrix jobs keep only the last wheel set per target. Add-py${{ matrix.python-version }}to each artifact name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-binaries.yml at line 202, The wheel artifact names in the build matrix are colliding across Python versions, so each upload should be uniquely named per target and python-version. Update the artifact name used by the upload step in the workflow to include the matrix python-version suffix, using the existing matrix fields such as matrix.python-version so Linux, musllinux, and musllinux-cross jobs preserve distinct wheel sets instead of overwriting each other.
204-206: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Show checkout-related sections and downstream containerized steps. git grep -nE 'actions/checkout@|persist-credentials|docker-run-action|run-on-arch-action' -- .github/workflows/build-binaries.yml echo echo '--- build-binaries.yml excerpt ---' nl -ba .github/workflows/build-binaries.yml | sed -n '180,380p'Repository: tower/tower-cli
Length of output: 1156
🏁 Script executed:
python3 - <<'PY' from pathlib import Path p = Path('.github/workflows/build-binaries.yml') for start, end in [(190, 235), (245, 285), (300, 340), (345, 375)]: print(f"\n--- lines {start}-{end} ---") for i, line in enumerate(p.read_text().splitlines(), 1): if start <= i <= end: print(f"{i:4d}: {line}") PYRepository: tower/tower-cli
Length of output: 7520
🌐 Web query:
uraimo/run-on-arch-action workspace mount repository checkout credentials persist-credentials💡 Result:
The uraimo/run-on-arch-action GitHub Action is designed to run commands within a container on non-x86 architectures [1][2]. It handles workspace mounting and repository access through standard GitHub Actions patterns rather than specific parameters within the action itself [3]. 1. Workspace Mount: The action automatically mounts the ${GITHUB_WORKSPACE} directory into the container [3]. If you need additional directories (like an artifacts directory), you must create them on the host using the setup property and then mount them into the container using the dockerRunArgs property [4][5][6]. Example: setup: | mkdir -p "${PWD}/artifacts" dockerRunArgs: | --volume "${PWD}/artifacts:/artifacts" 2. Repository Checkout: The repository is typically checked out using the standard actions/checkout action before calling run-on-arch-action [5]. 3. Persist Credentials: The persist-credentials setting is a parameter of the actions/checkout action, not run-on-arch-action [7]. When using actions/checkout, you can control whether the token is persisted in the local git configuration by setting persist-credentials: false (which is often recommended for security) [7]. Because run-on-arch-action mounts the ${GITHUB_WORKSPACE} (where the.git directory resides), any authentication settings or git configurations applied during the preceding checkout step remain available inside the container [3]. Important Note: When using run-on-arch-action, if you use the githubToken input to cache Docker images in your project's GitHub Package Registry, be aware that the cached image layer will be publicly available if your package registry is public [1][8]. Do not store secrets in the container state or logs, especially when using the install property [4][9].
Citations:
- 1: https://github.com/uraimo/run-on-arch-action/
- 2: https://github.com/marketplace/actions/run-on-architecture?version=v2.1.0
- 3: https://github.com/uraimo/run-on-arch-action/blob/master/src/run-on-arch.sh
- 4: https://github.com/uraimo/run-on-arch-action
- 5: https://github.com/uraimo/run-on-arch-action/blob/252d804c047d4d4d14fe923c46fddfdd333b093c/.github/workflows/advanced-example.yml
- 6: https://github.com/marketplace/actions/run-on-architecture
- 7: https://github.com/astropy/astropy/actions/runs/15058482899/workflow?pr=18155
- 8: https://github.com/uraimo/run-on-arch-action/blob/master/README.md
- 9: https://github.com/uraimo/run-on-arch-action/blob/master/action.yml
Disable persisted checkout credentials in the containerized build jobs.
musllinuxandmusllinux-crossboth pass the checked-out workspace intoaddnab/docker-run-action/uraimo/run-on-arch-action, so the default checkout token remains available inside the container.linux-crossalready setspersist-credentials: false; the same should be added here.Suggested change
- uses: actions/checkout@v6 with: submodules: recursive + persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.- uses: actions/checkout@v6 with: submodules: recursive persist-credentials: false🧰 Tools
🪛 zizmor (1.26.1)
[warning] 204-206: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/build-binaries.yml around lines 204 - 206, The checkout step for the containerized build jobs still leaves persisted credentials available in the workspace, unlike the existing linux-cross setup. Update the actions/checkout usage in the musllinux and musllinux-cross job paths to disable credential persistence by adding the same persist-credentials setting used elsewhere, so the checked-out token is not exposed to addnab/docker-run-action or uraimo/run-on-arch-action.Source: Linters/SAST tools
crates/tower-cmd/src/api.rs (1)
381-405: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Redact credential payloads before calling the generic response helper.
vend_catalog_credentialsnow sends a bearer-token response throughunwrap_api_response, whose debug path logs the raw response body. With--debug, this command will printcredentials.oauth_tokento stdout even when--show-tokenis omitted.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tower-cmd/src/api.rs` around lines 381 - 405, `vend_catalog_credentials` is passing a sensitive bearer-token response into `unwrap_api_response`, which can expose `credentials.oauth_token` in debug logs. Update `vend_catalog_credentials` (and, if needed, the `unwrap_api_response` call path) to redact or avoid logging the raw response body for this API before returning the `VendCatalogCredentialsResponse`, so debug output never prints the token unless explicitly allowed by the command.crates/tower-cmd/src/catalogs.rs (1)
241-247: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
ast-grep outline crates/tower-cmd/src/catalogs.rs --view expandedRepository: tower/tower-cli
Length of output: 1813
🏁 Script executed:
set -euo pipefail ast-grep outline crates/tower-cmd/src/catalogs.rs --view expandedRepository: tower/tower-cli
Length of output: 1813
🏁 Script executed:
set -euo pipefail sed -n '225,270p' crates/tower-cmd/src/catalogs.rs printf '\n--- tests ---\n' sed -n '560,650p' crates/tower-cmd/src/catalogs.rs printf '\n--- windows mentions ---\n' rg -n "windows|Windows|win32|cfg\\(windows\\)" -S .Repository: tower/tower-cli
Length of output: 50371
🏁 Script executed:
set -euo pipefail sed -n '144,360p' crates/tower-cmd/src/catalogs.rs printf '\n--- workspace/binary packaging ---\n' rg -n "wix|windows|cfg\\(windows\\)|target.*windows|tower-cmd" Cargo.toml crates -SRepository: tower/tower-cli
Length of output: 10490
Don't emit a POSIX-only token export snippet unconditionally.
token_export_commandrelies onexport, shell quoting, andpython3, so the hidden-token flow won't work for the Windows binaries shipped here. Render a platform-specific command or a manual set/copy instruction on Windows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/tower-cmd/src/catalogs.rs` around lines 241 - 247, The token export helper currently hardcodes a POSIX shell snippet in token_export_command, which won’t work for Windows binaries. Update token_export_command to detect the target platform and emit a platform-specific instruction: keep the existing export-based flow for Unix-like shells, but on Windows return a PowerShell-friendly or manual set/copy command instead of relying on export, shell quoting, and python3. Use the existing token_export_command symbol as the place to branch the output.src/tower/_context.py (1)
65-83: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Normalize blank auth env vars before session fallback.
With
TOWER_API_KEY=""orTOWER_JWT="", Line 75 treats auth as present and skips_read_session(), but downstream auth checks still treat the empty value as missing. This breaks thetower loginfallback for shells/CI that export empty vars.Proposed fix
def build(cls): + def getenv_or_none(name: str) -> Optional[str]: + return os.getenv(name) or None + session = {} - tower_url = os.getenv("TOWER_URL") + tower_url = getenv_or_none("TOWER_URL") tower_environment = ( os.getenv("TOWER__RUNTIME__ENVIRONMENT_NAME") or os.getenv("TOWER_ENVIRONMENT") or "default" ) - tower_api_key = os.getenv("TOWER_API_KEY") - tower_jwt = os.getenv("TOWER_JWT") - tower_run_id = os.getenv("TOWER__RUNTIME__RUN_ID") + tower_api_key = getenv_or_none("TOWER_API_KEY") + tower_jwt = getenv_or_none("TOWER_JWT") + tower_run_id = getenv_or_none("TOWER__RUNTIME__RUN_ID")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.def build(cls): def getenv_or_none(name: str) -> Optional[str]: return os.getenv(name) or None session = {} tower_url = getenv_or_none("TOWER_URL") tower_environment = ( os.getenv("TOWER__RUNTIME__ENVIRONMENT_NAME") or os.getenv("TOWER_ENVIRONMENT") or "default" ) tower_api_key = getenv_or_none("TOWER_API_KEY") tower_jwt = getenv_or_none("TOWER_JWT") tower_run_id = getenv_or_none("TOWER__RUNTIME__RUN_ID") if tower_url is None or (tower_api_key is None and tower_jwt is None): session = _read_session() if tower_url is None: tower_url = ( _get_session_value(session, "tower_url") or "https://api.tower.dev" ) if tower_api_key is None and tower_jwt is None:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tower/_context.py` around lines 65 - 83, Normalize empty auth environment values in the `_context.py` initialization path by treating blank `TOWER_API_KEY` and `TOWER_JWT` the same as unset before the `_read_session()` fallback. Update the `tower_api_key`/`tower_jwt` handling in the context-loading logic so `""` does not count as present, allowing `tower login` session values to be used when env vars are exported empty. Keep the existing `tower_url` and `tower_run_id` flow unchanged.src/tower/_tables.py (1)
87-97: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Re-enter credential refresh for existing
read-writecatalogs.After the first write,
_catalog_modebecomes"read-write", so later writes/drop/create calls return the existing catalog without callingget_tower_catalog()again. That bypasses_storage’sexpires_atrefresh logic and can leave long-running table handles using expired short-lived credentials. Consider a refresh path that re-entersget_tower_catalog()for vended write operations while letting the credential cache decide whether a new vend is needed.Also applies to: 486-498
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tower/_tables.py` around lines 87 - 97, The current read-write fast path in _ensure_read_write_table prevents re-entering get_tower_catalog() once _catalog_mode is already "read-write", which skips the _storage expires_at refresh for long-lived handles. Update the write/drop/create flow so existing read-write catalogs still go back through get_tower_catalog() when needed, while letting the credential cache decide whether to vend new credentials. Keep the fix centered on _ensure_read_write_table and the related read-write call sites so refreshed storage credentials are picked up before subsequent operations.
konstantinoscs
changed the title
Contributor
Author
|
@coderabbitai re-trigger the failed workflow |
konstantinoscs
requested review from
bradhe,
giray123,
jo-sm,
sammuti and
socksy
and removed request for
jo-sm
|
(╯°□°)╯︵ 🐇 ✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/tower-cmd/src/catalogs.rs`:
- Around line 241-247: The token export helper in token_export_command always
shells out to bare tower --json catalogs credentials, which discards any active
custom Tower endpoint used earlier. Update the generated export command to
preserve and reuse the current Tower URL context when fetching credentials, so
the TOWER_CATALOG_TOKEN comes from the same backend as the catalog metadata;
locate the fix in token_export_command and its format string.
- Around line 321-335: The hidden-token DBeaver snippet is using the wrong
variable syntax and will not resolve as emitted. Update the dbeaver_token branch
in crates/tower-cmd/src/catalogs.rs to use DBeaver’s ${...} placeholder format,
matching the existing hidden-token style used by sql_token and dbt_token, while
keeping the show_token path unchanged.
In `@src/tower/_context.py`:
- Around line 18-22: The session loading fallback in the context builder does
not treat undecodable session files as invalid data, so a corrupted session file
can still break context construction. Update the session read path in the
context-loading logic around the json.load(session_file) call to also catch
UnicodeDecodeError together with the existing file and JSON exceptions, and
return an empty session state in that case.
In `@src/tower/_storage.py`:
- Around line 67-73: Expired credential entries in the caching path are never
removed, so update the cache handling in the credential lookup flow around
_credential_cache, _CachedCredentials, and is_usable to evict any unusable entry
before storing the newly fetched credentials. In the branch that checks cached
credentials, remove the stale cache_key when cached.is_usable(now) is false,
then proceed to call _vend_with_default_catalog_fallback and store the fresh
_CachedCredentials as before.
In `@src/tower/_tables.py`:
- Around line 87-97: The write-escalation path in _ensure_read_write_table and
_ensure_catalog_mode can skip credential refresh once _catalog_mode is already
"read-write", which leaves long-lived Table/TableReference instances using stale
storage credentials. Update the read-write transition logic so write operations
still route through get_tower_catalog() or otherwise consult _storage for
refresh-before-expiry behavior, and avoid returning early solely because the
mode already matches. Use _ensure_read_write_table, _ensure_catalog_mode, and
get_tower_catalog() as the key places to adjust the refresh behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ffb1ff46-0477-4416-875a-8387a8db061c
📒 Files selected for processing (14)
crates/tower-cmd/src/api.rscrates/tower-cmd/src/catalogs.rscrates/tower-cmd/src/lib.rscrates/tower-cmd/src/mcp.rscrates/tower-cmd/src/templates/dbeaver.txt.tmplcrates/tower-cmd/src/templates/dbt.yml.tmplcrates/tower-cmd/src/templates/duckdb.sql.tmplcrates/tower-cmd/src/templates/pyiceberg.py.tmplcrates/tower-cmd/src/templates/spark.py.tmplsrc/tower/_context.pysrc/tower/_storage.pysrc/tower/_tables.pytests/tower/test_storage.pytests/tower/test_tables.py
konstantinoscs
force-pushed
the
feature/tow-2315-add-cli-commands-and-ux-for-storage-functionality
branch
from
0594a45 to
19c080a
Compare
Contributor
Author
|
@coderabbitai you're not functionaing well today. I've adressed all of your feedback, check and resolve the comments |
|
The first Comment image shows a terminal session on
The output shows the Rust dev profile finishing successfully, then the CLI printing:
The second Comment image shows another terminal session running:
The output again shows a successful Rust run, then details for the catalog:
Under
The preview values are partially redacted with
(；ω；)🐇 ✅ Action performedComments resolved. Approval is disabled; enable |
socksy
reviewed
Jun 29, 2026
| .long("type") | ||
| .value_parser(value_parser!(String)) | ||
| .help("Filter catalogs by type, e.g. tower-catalog") | ||
| .action(ArgAction::Set), |
Contributor
There was a problem hiding this comment.
now we have both clap::ArgAction::Set on line 23, and ArgAction::Set on this line - we should be consistent imo
| ) | ||
| } | ||
|
|
||
| fn print_credentials( |
Contributor
There was a problem hiding this comment.
this seems like a function better suited for output.rs?
| return; | ||
| } | ||
|
|
||
| print_credentials(name, &env, mode, &response.credentials, format, show_token); |
Contributor
There was a problem hiding this comment.
doesn't seem to be any error logic if the credentials are failed to be fetched?
| ) | ||
| .await; | ||
|
|
||
| if output::get_output_mode().is_json() { |
Contributor
There was a problem hiding this comment.
this is a smell that the print_credentials should be in output.rs and then takes care of whether to print out json inside of it to keep this irrelevant logic out of this file
| try: | ||
| with session_path.open() as session_file: | ||
| session = json.load(session_file) | ||
| except (FileNotFoundError, OSError, json.JSONDecodeError, UnicodeDecodeError): |
Contributor
There was a problem hiding this comment.
eh, why special case these errors if we're not differentiating between them?
| ) | ||
|
|
||
| if tower_api_key is None and tower_jwt is None: | ||
| tower_jwt = _session_jwt(session) |
Contributor
There was a problem hiding this comment.
Something seems super complicated here in all this logic getting session etc - IIRC in the cli we read from the env var, then the .config/tower/ session.json in turn, and state is just held by the fact that the latter can change (the former cannot, but TOWER_API_KEY isn't typically used in the scenario where that matters, which is a long running process — atm that's only the MCP server that e.g. needs to know that the team token changes when it changes team.
Here I feel it's already quite complicated (why do we have a context object with a build method? Doesn't feel pythonic at all to me) and on top of that I don't know why we need to support the sessions — is it expected that e.g. changing team in the CLI should somehow affect the local SDK? Would there be a scenario in which we want someone to setup this SDK and expect them to use the CLI session?
| return | ||
|
|
||
|
|
||
| def _unwrap_vend_result( |
Contributor
There was a problem hiding this comment.
why not just rely on the existing 404 error? needing to do this adds this function, and the need for _is_not_found() one-liner. Assuming there's an AI agent involved, can i maybe interest you in my simpler agent skill? https://github.com/socksy/agent-skills
| table, | ||
| table_reference=self if self._tower_vended else None, | ||
| table_identifier=table_name, | ||
| catalog_mode=self._catalog_mode, |
Contributor
There was a problem hiding this comment.
won't this always be "read-write" since you just ensured it above?
|
|
||
| namespace = namespace_or_default(self._namespace) | ||
| table_name = make_table_name(self._name, namespace) | ||
| catalog = self._ensure_catalog_mode("read-write") |
Contributor
There was a problem hiding this comment.
wouldn't it rather make sense to do this in the constructor function where we assign the catalog into the object? Seems weird to me to need to make the caller site do a sanitization when i would just expect to do things on the instance variables of the model? It doesn't seem there's any error in this path that you'd want to be lazily done
sammuti
reviewed
Jun 29, 2026
| URI: __TOWER_URI__ | ||
| Warehouse: __TOWER_WAREHOUSE__ | ||
| Authentication: Bearer token | ||
| Token: __TOWER_TOKEN__ |
Contributor
There was a problem hiding this comment.
I believe you need a scope field here
| @@ -0,0 +1,9 @@ | |||
| duckdb <<SQL | |||
Contributor
There was a problem hiding this comment.
Duckdb can also do vended credential flow on the terminal
Something like
duckdb <<SQL
INSTALL httpfs;
LOAD httpfs;
INSTALL iceberg;
LOAD iceberg;
SET s3_region='eu-central-1';
CREATE OR REPLACE SECRET tower_cat (
TYPE iceberg,
CLIENT_ID '__TOWER_CLIENT_ID__',
CLIENT_SECRET '__TOWER_CLIENT_SECRET__',
OAUTH2_SCOPE 'PRINCIPAL_ROLE:ALL',
ENDPOINT '__TOWER_URI__'
);
ATTACH '__TOWER_WAREHOUSE__' AS __TOWER_NAME__ (
TYPE iceberg,
SECRET tower_cat,
DEFAULT_REGION 'eu-central-1',
ACCESS_DELEGATION_MODE 'vended_credentials'
);
SQL
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds Tower-vended catalog credentials to the CLI and Python SDK so users can access Tower-managed Iceberg catalogs without manually configuring PyIceberg credentials.
This introduces a vend-first path for
tower.tables(), addstower catalogs credentials, and improves catalog discovery with storage-focused filters.Changes
tower.tables()so string catalog names use Tower credential vending by default.readcredentials first, while writes re-vendread-writecredentials and reload the table before writing.Catalogobjects working for tests and advanced/custom usage.tower_credentials=Falserollback path for legacy PyIceberg config loading.tower catalogs list --type <type>andtower catalogs list --storage.tower catalogs credentials <name>with snippets for:TOWER_CATALOG_TOKENinstead.--show-tokenfor explicit token printing and--jsonfor raw machine-readable output.New commands and command variations:
New sdk behavior:
Some local testing:
List storage catalogs only (catalog type == tower-catalog)
Show a catalog:
Vend credentials for a catalog:
Summary by CodeRabbit
New Features
catalogs credentialscommand to generate short-lived catalog credentials and ready-to-use connection snippets.catalogs list, including a new--typeoption and a--storageshortcut.Bug Fixes