Migrate existing run quickstart scripts to dev customer id
#473
Conversation
WalkthroughThis pull request updates the handling of Google Cloud settings in Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant R as run.py
participant E as Environment (CLOUD_PROVIDER)
U->>R: Execute script
R->>E: Check CLOUD_PROVIDER
E-->>R: Return provider info
alt GCP enabled
R->>R: Run GCP-specific logic
else
R->>R: Run standard logic
end
Possibly related PRs
Suggested reviewers
Poem
Warning Review ran into problems🔥 ProblemsGitHub Actions and Pipeline Checks: Resource not accessible by integration - https://docs.github.com/rest/actions/workflow-runs#list-workflow-runs-for-a-repository. Please grant the required permissions to the CodeRabbit GitHub App under the organization or repository settings. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/distribution/run_zipline_quickstart.sh (1)
23-23: Add error handling for directory change.Add failure handling to cd command.
- cd cananry-confs + cd cananry-confs || exit 1🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 23-23: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
api/py/ai/chronon/repo/run.py(3 hunks)scripts/distribution/build_and_upload_artifacts.sh(1 hunks)scripts/distribution/run_zipline_quickstart.sh(2 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
scripts/distribution/run_zipline_quickstart.sh
[warning] 23-23: Use 'cd ... || exit' or 'cd ... || return' in case cd fails.
(SC2164)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: non_spark_tests
- GitHub Check: non_spark_tests
- GitHub Check: enforce_triggered_workflows
🔇 Additional comments (11)
scripts/distribution/build_and_upload_artifacts.sh (1)
187-187: Added trailing slash for path consistency.Added trailing slash ensures consistent path handling when joining paths.
scripts/distribution/run_zipline_quickstart.sh (8)
17-18: Environment migration todev- BQ table references updated.Tables renamed to use
devsuffix instead oftest.
26-27: Updated git branch references fordevenvironment.Branch changed from
selectstodev.
34-34: Updated storage bucket fordevenvironment.Changed from
canarytodevbucket.
66-66: Updated config path to usedevenvironment.Changed from
purchases.v1_testtopurchases.v1_dev.
72-72: Updated config path for upload mode.Consistent with other changes to use
devenvironment.
79-79: Updated config path for upload-to-kv mode.Using consistent
devenvironment reference.
85-85: Updated config path for metadata-upload mode.Dev environment consistency maintained.
92-92: Updated config name for fetch mode.Changed to use
v1_devsuffix.api/py/ai/chronon/repo/run.py (2)
40-40: Added constant for combined args.New constant combines offline and online arguments.
820-824: Removedgcpparameter from signature.Parameter removed as part of migration to environment variable configuration.
| os.makedirs(ZIPLINE_DIRECTORY, exist_ok=True) | ||
|
|
||
| if dataproc or gcp: | ||
| if dataproc or os.environ.get('CLOUD_PROVIDER', '') == 'GCP': |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Environment-based cloud provider detection.
Replaced flag parameter with environment variable check.
🏁 Script executed:
#!/bin/bash
# Check if there are any remaining references to the removed --gcp flag
grep -r "\-\-gcp" . --include="*.py" --include="*.sh"Length of output: 591
Review Update: Verify update consistency in environment checks.
- In
api/py/ai/chronon/repo/run.py(line 833), using the environment variable for GCP detection is correct. - However, references to
--gcpstill exist inscripts/distribution/build_and_upload_artifacts.sh. Please confirm if these should be updated as well for consistency.
tchow-zlai
left a comment
tchow-zlai
left a comment
There was a problem hiding this comment.
are we no longer keeping canary around?
|
we are keeping |
## Summary ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [x] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Cloud settings are now auto-detected via environment variables rather than requiring a manual flag. - The quickstart deployment has been updated to align with the development environment, ensuring consistent operational behavior. - **Chores** - Improved cloud storage path formatting to enhance the reliability of file uploads. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [x] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Cloud settings are now auto-detected via environment variables rather than requiring a manual flag. - The quickstart deployment has been updated to align with the development environment, ensuring consistent operational behavior. - **Chores** - Improved cloud storage path formatting to enhance the reliability of file uploads. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [x] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Cloud settings are now auto-detected via environment variables rather than requiring a manual flag. - The quickstart deployment has been updated to align with the development environment, ensuring consistent operational behavior. - **Chores** - Improved cloud storage path formatting to enhance the reliability of file uploads. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [x] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Cloud settings are now auto-detected via environment variables rather than requiring a manual flag. - The quickstart deployment has been updated to align with the development environment, ensuring consistent operational behavior. - **Chores** - Improved cloud storage path formatting to enhance the reliability of file uploads. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
…#473) ## Summary ## Cheour clientslist - [ ] Added Unit Tests - [ ] Covered by existing CI - [x] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Cloud settings are now auto-detected via environment variables rather than requiring a manual flag. - The quiour clientsstart deployment has been updated to align with the development environment, ensuring consistent operational behavior. - **Chores** - Improved cloud storage path formatting to enhance the reliability of file uploads. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Checklist
Summary by CodeRabbit