ck followups #667
ck followups #667
Conversation
WalkthroughThis update removes all references to the Changes
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. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (20)
🪧 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: 0
🧹 Nitpick comments (5)
api/python/ai/chronon/resources/gcp/sources/test/data.py (1)
15-21: Consider adding validation for stream processing.When
topic=None, you may want to add a comment about implications for real-time processing and how users should modify for streaming sources.api/python/ai/chronon/resources/gcp/README.md (4)
111-121: Fix heading punctuation.Heading should not end with a period.
-## 🧪 Running a GroupBy upload (GBU) job. +## 🧪 Running a GroupBy upload (GBU) job🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
111-111: Trailing punctuation in heading
Punctuation: '.'(MD026, no-trailing-punctuation)
124-131: Fix heading punctuation.Heading should not end with a period.
-## 🧪 Upload the GBU values to online KV store. +## 🧪 Upload the GBU values to online KV store🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
124-124: Trailing punctuation in heading
Punctuation: '.'(MD026, no-trailing-punctuation)
135-149: Fix heading punctuation.Heading should not end with a period.
-## 🧪 Upload the metadata of Chronon GroupBy or Join to online KV store for serving. +## 🧪 Upload the metadata of Chronon GroupBy or Join to online KV store for serving🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
135-135: Trailing punctuation in heading
Punctuation: '.'(MD026, no-trailing-punctuation)
153-173: Fix heading and formatting issues.The heading shouldn't end with a period, and there should be no spaces inside emphasis markers.
-## 🧪 Fetch feature values from Chronon GroupBy or Join. +## 🧪 Fetch feature values from Chronon GroupBy or Join -** Note:** This is only for debugging purposes. Not for production use. +**Note:** This is only for debugging purposes. Not for production use.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
153-153: Trailing punctuation in heading
Punctuation: '.'(MD026, no-trailing-punctuation)
155-155: Spaces inside emphasis markers
null(MD037, no-space-in-emphasis)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (5)
api/python/ai/chronon/repo/run.py(0 hunks)api/python/ai/chronon/resources/gcp/README.md(2 hunks)api/python/ai/chronon/resources/gcp/sources/data.py(0 hunks)api/python/ai/chronon/resources/gcp/sources/test/data.py(1 hunks)api/python/ai/chronon/resources/gcp/teams.py(4 hunks)
💤 Files with no reviewable changes (2)
- api/python/ai/chronon/resources/gcp/sources/data.py
- api/python/ai/chronon/repo/run.py
🧰 Additional context used
🧬 Code Graph Analysis (1)
api/python/ai/chronon/resources/gcp/sources/test/data.py (4)
api/src/main/scala/ai/chronon/api/ParametricMacro.scala (1)
api(66-66)api/src/main/scala/ai/chronon/api/Builders.scala (1)
Source(106-140)api/src/main/scala/ai/chronon/api/Extensions.scala (3)
query(350-358)table(387-387)topic(414-424)api/python/ai/chronon/query.py (1)
selects(92-111)
🪛 markdownlint-cli2 (0.17.2)
api/python/ai/chronon/resources/gcp/README.md
111-111: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
124-124: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
135-135: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
153-153: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
155-155: Spaces inside emphasis markers
null
(MD037, no-space-in-emphasis)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: service_tests
- GitHub Check: service_tests
- GitHub Check: service_commons_tests
- GitHub Check: online_tests
- GitHub Check: hub_tests
- GitHub Check: hub_tests
- GitHub Check: cloud_gcp_tests
- GitHub Check: flink_tests
- GitHub Check: cloud_aws_tests
- GitHub Check: cloud_gcp_tests
- GitHub Check: online_tests
- GitHub Check: cloud_aws_tests
- GitHub Check: aggregator_tests
- GitHub Check: api_tests
- GitHub Check: api_tests
- GitHub Check: aggregator_tests
- GitHub Check: orchestration_tests
- GitHub Check: orchestration_tests
- GitHub Check: flink_tests
🔇 Additional comments (9)
api/python/ai/chronon/resources/gcp/teams.py (3)
27-33: Good addition of clear TODOs for required configuration.The added placeholders with TODO comments make required configuration values explicit.
41-42: Clear indication of required customer configuration.Good use of TODO comment to highlight fields needing customer-specific values.
53-54:✅ Verification successful
Verify outputNamespace swap impact.
The outputNamespace values have been swapped between the test and team_conf Team objects. Ensure this doesn't break existing code or tests.
Also applies to: 64-65
🏁 Script executed:
#!/bin/bash # Check for references to these namespaces echo "Looking for uses of 'test' and 'data' namespaces..." rg -n "'test'|\"test\"" --type py | grep -v "resources/gcp/teams.py" rg -n "'data'|\"data\"" --type py | grep -v "resources/gcp/teams.py"Length of output: 1752
No impact from swapped outputNamespace values
All occurrences of"test"/"data"are in tests (which explicitly setoutputNamespace), and no production code references the old defaults. This swap won’t break existing code or tests.api/python/ai/chronon/resources/gcp/sources/test/data.py (1)
1-24: Well-structured source example with clear documentation.The example effectively demonstrates creating a Chronon Source from a batch table with appropriate comments.
api/python/ai/chronon/resources/gcp/README.md (5)
31-32: Clear CLI installation instruction.Good clarification that Zipline CLI is only needed for upgrades/downgrades.
47-68: Improved team configuration guidance.Clearer explanation of team configuration requirements with specific examples.
69-75: Helpful partition configuration explanation.Explicit instructions for date partitioning configuration requirements.
79-85: Simplified compilation instructions.Clear command for generating configurations from Python objects.
89-108: Streamlined backfill instructions.Concise CLI commands for running GroupBy and Join backfills.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
api/python/ai/chronon/resources/gcp/README.md (5)
46-49: Adjust duplicate Requirements heading.
Rename or demote the second “## Requirements” to avoid repeating the same heading.
98-104: Remove trailing period in heading.
Drop the final “.” after “## 🧪 Running a GroupBy upload (GBU) job” per markdown style.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
98-98: Trailing punctuation in heading
Punctuation: '.'(MD026, no-trailing-punctuation)
111-118: Remove trailing period in heading.
Drop the final “.” after “## 🧪 Upload the GBU values to online KV store” per markdown style.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
111-111: Trailing punctuation in heading
Punctuation: '.'(MD026, no-trailing-punctuation)
121-128: Remove trailing period in heading.
Drop the final “.” after “## 🧪 Upload the metadata of Chronon GroupBy or Join to online KV store for serving” per markdown style.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
122-122: Trailing punctuation in heading
Punctuation: '.'(MD026, no-trailing-punctuation)
139-160: Remove trailing period in heading.
Drop the final “.” after “## 🧪 Fetch feature values from Chronon GroupBy or Join” per markdown style.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
140-140: Trailing punctuation in heading
Punctuation: '.'(MD026, no-trailing-punctuation)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (1)
api/python/ai/chronon/resources/gcp/README.md(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
api/python/ai/chronon/resources/gcp/README.md
98-98: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
111-111: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
122-122: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
140-140: Trailing punctuation in heading
Punctuation: '.'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (21)
- GitHub Check: service_commons_tests
- GitHub Check: service_tests
- GitHub Check: cloud_gcp_tests
- GitHub Check: hub_tests
- GitHub Check: cloud_aws_tests
- GitHub Check: cloud_gcp_tests
- GitHub Check: service_tests
- GitHub Check: cloud_aws_tests
- GitHub Check: online_tests
- GitHub Check: online_tests
- GitHub Check: hub_tests
- GitHub Check: flink_tests
- GitHub Check: api_tests
- GitHub Check: api_tests
- GitHub Check: flink_tests
- GitHub Check: aggregator_tests
- GitHub Check: aggregator_tests
- GitHub Check: orchestration_tests
- GitHub Check: orchestration_tests
- GitHub Check: python_tests
- GitHub Check: enforce_triggered_workflows
🔇 Additional comments (6)
api/python/ai/chronon/resources/gcp/README.md (6)
31-31: Approve Zipline CLI entry.
58-59: Approve partition config snippet.
Sample keys and comments are clear and well formatted.
64-69: Approve Compiling instructions.
Commands and context are concise and correct.
76-92: Approve backfill CLI workflows.
GroupBy and Join backfill steps are clear and consistent.
94-96: Approve backfill result note.
Description and separator are fine.
107-107: Approve upload result note.
Output destination description looks good.
## Summary ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Revised GCP integration guide for improved clarity, streamlined instructions, and added new sections on metadata and KV store uploads, as well as debugging feature fetching. - **Bug Fixes** - Corrected configuration in team settings by swapping output namespaces for the default teams. - **Chores** - Removed outdated example source configuration and replaced it with an updated, clearer example in a dedicated test location. - Cleaned up configuration placeholders and added TODO comments for required user input. - **Refactor** - Removed the deprecated dataproc option from the command-line interface and related configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Revised GCP integration guide for improved clarity, streamlined instructions, and added new sections on metadata and KV store uploads, as well as debugging feature fetching. - **Bug Fixes** - Corrected configuration in team settings by swapping output namespaces for the default teams. - **Chores** - Removed outdated example source configuration and replaced it with an updated, clearer example in a dedicated test location. - Cleaned up configuration placeholders and added TODO comments for required user input. - **Refactor** - Removed the deprecated dataproc option from the command-line interface and related configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Revised GCP integration guide for improved clarity, streamlined instructions, and added new sections on metadata and KV store uploads, as well as debugging feature fetching. - **Bug Fixes** - Corrected configuration in team settings by swapping output namespaces for the default teams. - **Chores** - Removed outdated example source configuration and replaced it with an updated, clearer example in a dedicated test location. - Cleaned up configuration placeholders and added TODO comments for required user input. - **Refactor** - Removed the deprecated dataproc option from the command-line interface and related configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary ## Checklist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Revised GCP integration guide for improved clarity, streamlined instructions, and added new sections on metadata and KV store uploads, as well as debugging feature fetching. - **Bug Fixes** - Corrected configuration in team settings by swapping output namespaces for the default teams. - **Chores** - Removed outdated example source configuration and replaced it with an updated, clearer example in a dedicated test location. - Cleaned up configuration placeholders and added TODO comments for required user input. - **Refactor** - Removed the deprecated dataproc option from the command-line interface and related configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
## Summary ## Cheour clientslist - [ ] Added Unit Tests - [ ] Covered by existing CI - [ ] Integration tested - [ ] Documentation update <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Documentation** - Revised GCP integration guide for improved clarity, streamlined instructions, and added new sections on metadata and KV store uploads, as well as debugging feature fetching. - **Bug Fixes** - Corrected configuration in team settings by swapping output namespaces for the default teams. - **Chores** - Removed outdated example source configuration and replaced it with an updated, clearer example in a dedicated test location. - Cleaned up configuration placeholders and added TODO comments for required user input. - **Refactor** - Removed the deprecated dataproc option from the command-line interface and related configuration. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary
Checklist
Summary by CodeRabbit
Documentation
Bug Fixes
Chores
Refactor