Skip to content

Conversation

@bradhe
Copy link
Contributor

@bradhe bradhe commented May 13, 2025

  • Inject catalogs in local runtime environment
  • Update behavior of wait_for_run and add wait_for_runs to detect failure
  • Updated documentation
  • Bump version to v0.3.14

bradhe and others added 15 commits May 9, 2025 15:00
…ments-for-table-class-and-tables-helper

Updated docstrings in _tables
…e-not-initialized-in-local-run-mode

Inject catalogs in local runtime environment
* fix: Fix type mappings, format code

* feat: Handle nested types correctly and set ids

* chore: Fix formatting

* chore: Add how to run tests

* feat: Add tests for types
Updated implementations ensure that we equally check runs to detect failures
part way through executions. Likewise, we add timeouts while talking to the
Tower API in case there are some operational problems on that side of things.
Thanks @copilot for the recommendation!
Thanks @copilot

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…s-to-take-a-list-of-runs-or-a-single-run

Add a simple `wait_for_runs` function
@bradhe bradhe requested review from Copilot and sankroh May 13, 2025 13:07
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR ultimately updates the Tower runtime’s API‐client behavior and test coverage, adds catalog injection in the local runtime environment, and enhances error handling and documentation across several modules.

  • Updates test cases to validate new run-waiting behaviors
  • Introduces catalog fetching and injection in the CLI runtime
  • Enhances documentation and API interfaces in both Python and Rust components

Reviewed Changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/tower/test_client.py Updated test fixtures and added tests for wait_for_run and wait_for_runs behavior
src/tower/utils/pyarrow.py Refactored type conversion with new field ID management
src/tower/exceptions.py Added new exception types for improved error clarity
src/tower/_tables.py Updated documentation and method signatures for table operations
src/tower/_client.py Introduced wait_for_runs, enhanced polling logic, and detailed argument documentation
src/tower/init.py Exported wait_for_runs for public API
pyproject.toml Version remains outdated relative to the PR title
crates/tower-cmd/** Added catalog fetching/injection and improved error handling in CLI commands
crates/crypto/** Updated encryption/decryption functions to return Results and use robust error handling
README.md Extended test instructions for better developer guidance
Cargo.toml Minor dependency updates; version number still reflects previous release
Comments suppressed due to low confidence (1)

pyproject.toml:7

  • The project version in pyproject.toml still shows '0.3.13' despite the PR title indicating a v0.3.14 release. Update the version number for consistency.
version = "0.3.13"

os.environ["TOWER_URL"] = "https://api.example.com"
os.environ["TOWER_API_KEY"] = "abc123"
tower = mock_api_config
run = create_run_object(number=run_number, status="crashed")
Copy link

Copilot AI May 13, 2025

Choose a reason for hiding this comment

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

The test initializes a run object with a 'crashed' status while the mocked API responses simulate a transition from 'pending' to 'exited'. Consider initializing the run with 'pending' for clarity and consistency with the expected workflow.

Suggested change
run = create_run_object(number=run_number, status="crashed")
run = create_run_object(number=run_number, status="pending")

Copilot uses AI. Check for mistakes.
# We need to re-add this run to the list so we check it again
# in the future. We add it to the back since we took it off the
# front, effectively moving to the next run.
awaiting_runs.append(run)
Copy link

Copilot AI May 13, 2025

Choose a reason for hiding this comment

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

[nitpick] When polling run status, consider appending the updated run object (e.g., the 'desc' returned from _check_run_status) instead of the original 'run', to ensure subsequent checks reflect the latest state.

Suggested change
awaiting_runs.append(run)
awaiting_runs.append(desc)

Copilot uses AI. Check for mistakes.
@bradhe bradhe requested a review from Copilot May 13, 2025 13:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR prepares the v0.3.14 release by injecting catalogs into the local runtime, updating the polling logic for run completion (including a new wait_for_runs function), enhancing documentation across the codebase, and bumping the version.

  • Inject catalogs into the local runtime environment
  • Update wait_for_run behavior and add wait_for_runs for improved run status detection
  • Update documentation and version numbers across modules

Reviewed Changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/tower/test_client.py Updated tests for new fixtures and run-waiting behavior
src/tower/utils/pyarrow.py Adjusted conversion functions and updated nested type conversion logic
src/tower/exceptions.py Added new exception classes
src/tower/_tables.py Expanded docstrings and improved schema/table methods documentation
src/tower/_client.py Refactored run polling functions and added wait_for_runs
crates/tower-cmd/src/(secrets.rs, run.rs, lib.rs, error.rs, api.rs) Updated secret encryption/decryption and runtime command error handling changes
crates/crypto Updated cryptography functions to use AES and improved error handling
pyproject.toml & Cargo.toml Bumped version numbers to v0.3.14
Comments suppressed due to low confidence (2)

src/tower/_client.py:158

  • The variable 'WAIT_TIMEOUT' is used in wait_for_run() but is not defined in this module. Please define or import it as a constant to prevent runtime errors.
time.sleep(WAIT_TIMEOUT)

src/tower/_client.py:235

  • The same undefined 'WAIT_TIMEOUT' is used in wait_for_runs(). Please ensure this constant is declared or imported for consistent behavior across run polling functions.
time.sleep(WAIT_TIMEOUT)

@bradhe bradhe merged commit e1bcdf9 into main May 13, 2025
20 checks passed
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.

4 participants