-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: update publish workflows, removing old workflow, tweaking main workflow for usability, and updating release docs #151
Conversation
Warning Rate limit exceeded@aaronsteers has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 40 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 Walkthrough📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve the removal of the Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
What do you think about the suggested reviewers? Do they seem like a good fit for this PR? Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/pypi_publish.yml (1)
17-17
: Consider formatting the version description for better readability?The description is quite informative but a bit dense. What do you think about breaking it into bullet points for better scanning? Something like:
- description: "Version. For public-facing stable releases, please use the GitHub Releases workflow instead: https://github.com/airbytehq/airbyte-python-cdk/blob/main/docs/RELEASES.md. If running this workflow again main or from a dev branch, the desired version number is required to be entered here, e.g. `v1.2.3dev0` or `v1.2.3rc1`." + description: | + Version number for the release. Notes: + • For public-facing stable releases: Use GitHub Releases workflow instead (see docs/RELEASES.md) + • For main/dev branch releases: Version number required (e.g. `v1.2.3dev0` or `v1.2.3rc1`)wdyt? 🤔
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
.github/workflows/publish_sdm_connector.yml
(0 hunks).github/workflows/pypi_publish.yml
(2 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/publish_sdm_connector.yml
🔇 Additional comments (4)
.github/workflows/pypi_publish.yml (4)
33-33
: LGTM on the default value change!
Changing update_connector_builder
default to false
makes sense as it's a less frequent operation that should be explicitly opted into. This helps prevent accidental updates.
Line range hint 185-191
: Multi-platform build configuration looks good!
The Docker build configuration properly handles both amd64 and arm64 architectures. Nice touch on using QEMU for multi-platform builds! 👍
Line range hint 41-91
: Version handling logic looks robust!
The version validation and detection logic handles all edge cases well:
- Git tag detection
- Input validation
- Version format checking
- Prerelease detection
Let's verify the version validation behavior:
✅ Verification successful
The previous search only found a dependency version constraint. Let's look specifically at workflow files and version-related functions to ensure our version handling is consistent across the codebase.
Version handling logic is solid and consistent! 🎯
The version validation in the workflow handles all scenarios perfectly:
- Correctly strips 'v' prefix from both git tags and manual inputs
- Validates version format matches
X.Y.Z
- Properly handles prerelease versions
- Maintains consistency across PyPI and Docker publishing
- Includes safeguards against version conflicts
No conflicting version patterns or inconsistent handling was found in the codebase. The implementation aligns well with the project's release workflow.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Test version validation logic with different formats
# Looking for any hardcoded version patterns that might conflict
rg -g '!*.md' -g '!*.txt' 'v\d+\.\d+\.\d+' .
Length of output: 202
Script:
#!/bin/bash
# Let's check for any version-related patterns in GitHub workflow files
rg -g '*.yml' -g '*.yaml' 'version|tag|ref' .github/workflows/
# Also check for any version validation or handling in Python files
ast-grep --pattern 'def $_(version$_)'
Length of output: 9620
Line range hint 251-261
: Shall we verify the file paths in the Connector Builder update?
The update logic looks solid with proper retries, but let's verify the file paths are correct:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
docs/RELEASES.md (3)
Line range hint
3-15
: LGTM! Nice clarity improvement with the SDM inclusion! 🎉The steps are well-structured and clear. Would you consider adding a brief note about what SDM stands for (Source-Declarative-Manifest) for newcomers to the project? wdyt?
🧰 Tools
🪛 Markdownlint (0.35.0)
5-5: null
Bare URL used(MD034, no-bare-urls)
16-16
: How about formatting this as a proper markdown link? 🔗For better readability and maintainability, would you consider reformatting the bare URL as a markdown link? Something like:
-You can monitor the PyPI release process here in the GitHub Actions view: https://github.com/airbytehq/airbyte-python-cdk/actions/workflows/pypi_publish.yml +You can monitor the PyPI release process in the [GitHub Actions view](https://github.com/airbytehq/airbyte-python-cdk/actions/workflows/pypi_publish.yml)🧰 Tools
🪛 Markdownlint (0.35.0)
16-16: null
Bare URL used(MD034, no-bare-urls)
20-28
: Love the simplified process! ✨The steps are much clearer now. Would it be helpful to add a note about version number format requirements (e.g., PEP 440 compliance) before the examples? This could help prevent version formatting issues. wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
docs/RELEASES.md
(2 hunks)
🧰 Additional context used
🪛 Markdownlint (0.35.0)
docs/RELEASES.md
16-16: null
Bare URL used
(MD034, no-bare-urls)
18-18: Expected: asterisk; Actual: underscore
Emphasis style
(MD049, emphasis-style)
18-18: Expected: asterisk; Actual: underscore
Emphasis style
(MD049, emphasis-style)
🔇 Additional comments (1)
docs/RELEASES.md (1)
18-18
: LGTM! Great addition of the video walkthrough! 📹
🧰 Tools
🪛 Markdownlint (0.35.0)
18-18: Expected: asterisk; Actual: underscore
Emphasis style
(MD049, emphasis-style)
18-18: Expected: asterisk; Actual: underscore
Emphasis style
(MD049, emphasis-style)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the keeping the docs so up-to-date!
Summary by CodeRabbit
Bug Fixes
New Features
update_connector_builder
input tofalse
in the PyPI publish workflow.Documentation
These changes improve the clarity and functionality of the publishing workflows, ensuring a smoother experience for users managing package releases.