CI: Implement automatic dynamic versioning for pre-releases#469
Conversation
There was a problem hiding this comment.
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
.github/workflows/pypi_publish.yml:8
- [nitpick] The step name 'CDK Publish' may be ambiguous if the repository is not entirely focused on CDK-related projects. Consider renaming it to 'Publish Python Package' or another clear descriptor to improve clarity.
+name: CDK Publish
📝 WalkthroughWalkthroughThis pull request revises the Continuous Deployment workflow and versioning configuration. In the GitHub Actions workflow file, the workflow name has been updated, the tag-based trigger is now commented out, and several steps have been added or reordered—including checking out the CDK repository and detecting versions using Dunamai. Additionally, version detection and validation steps have been modified with clearer parameter outputs. In the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
What do you think about the suggested reviewers? Do they align with your expectations? 🪧 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.
Actionable comments posted: 0
🧹 Nitpick comments (2)
.github/workflows/pypi_publish.yml (2)
52-61: Git Reference-Based Version Override:
Introducing the “Detect Release Tag Version from git ref (‘${{ github.ref_name }}’)” step helps ensure that, when a tag is present, the version number is directly extracted from the git reference—overriding the Dunamai result if necessary. This fallback mechanism increases robustness. Would you consider checking for possible edge cases (for example, when the ‘v’ prefix is unexpectedly absent) to make the extraction even more resilient? wdyt?
63-91: Version Validation and Setting Logic:
The “Validate and set VERSION” step thoroughly handles cases such as empty inputs, mismatches between detected and input versions, and even determines if a version qualifies as a prerelease using a regex. This level of validation should help maintain consistency in version outputs. One thought: would you consider refining the regex check to more strictly validate against the full PEP 440 specification (especially for post or dev releases), or is the current implementation sufficient for our needs? wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/pypi_publish.yml(2 hunks)pyproject.toml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Check: 'source-pokeapi' (skip=false)
- GitHub Check: Check: 'source-amplitude' (skip=false)
- GitHub Check: Check: 'source-shopify' (skip=false)
- GitHub Check: Check: 'source-hardcoded-records' (skip=false)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (Fast)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: SDM Docker Image Build
🔇 Additional comments (5)
pyproject.toml (1)
29-30: PEP 440 Dynamic Versioning Configuration:
The added linestyle = "pep440" # Ensures compatibility with PyPIin the
[tool.poetry-dynamic-versioning]section clearly instructs Poetry to use a PEP 440 compliant versioning scheme. This nicely aligns with the PR objective of ensuring dynamic versioning adheres to PyPI standards. wdyt?.github/workflows/pypi_publish.yml (4)
8-8: Workflow Renaming for Clarity:
Renaming the workflow to “CDK Publish” helps clarify its purpose within the CI process. This makes it more self-explanatory for maintainers and contributors. wdyt?
12-13: Tag Trigger Adjustment:
Commenting out the tag trigger (i.e.,# tags: # - "v*" ```) is a deliberate choice to avoid unintended workflow executions on tag pushes. Could you confirm that this change meets the release workflow requirements as intended? wdyt? --- `41-45`: **CDK Repository Checkout:** Adding the “Checkout CDK Repo” step with a full fetch (`fetch-depth: 0`) ensures that the complete commit history is available for Dunamai to compute the versioning accurately. This is especially important for dynamic versioning based on commit distance. Looks good—wdyt? --- `46-51`: **Dunamai Prerelease Version Detection:** The new step to “Detect Prerelease Version using Dunamai” employs the Dunamai action with the `--style pep440` argument, directly aligning with the updated versioning style. This integration should reliably compute the prerelease version as planned. wdyt? </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
.github/workflows/pypi_publish.yml (3)
51-56: Dunamai Version Detection Integration
Integrating the Dunamai action with the argument--style pep440effectively enforces PEP 440 compliance on the detected version. Have you considered adding additional error logging or handling if Dunamai fails to detect a version properly? wdyt?
57-67: Release Tag Version Override Logic
The logic to override the Dunamai-detected version when a tag push is detected is clear and properly strips the 'v' prefix. Would you consider enhancing this step with a regex validation post-stripping to ensure strict adherence to PEP 440? wdyt?
68-103: Robust Version Validation and Setting
The script to validate and set theVERSIONis well-structured—it handles both the absence of inputs and the case where user input conflicts with the detected version. As an extra precaution, would you consider logging more details when a version mismatch occurs or refining the prerelease detection regex to cover additional edge cases (such as pre-release identifiers like "rc", "a", or "b")? wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/pypi_publish.yml(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Check: 'source-pokeapi' (skip=false)
- GitHub Check: Check: 'source-amplitude' (skip=false)
- GitHub Check: Check: 'source-shopify' (skip=false)
- GitHub Check: Check: 'source-hardcoded-records' (skip=false)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: Pytest (Fast)
🔇 Additional comments (3)
.github/workflows/pypi_publish.yml (3)
8-8: Name Update: Workflow renamed to "CDK Publish"
The new workflow name clearly aligns with the dynamic versioning strategy and the overall CI improvements. Is this naming consistent with your broader release process? wdyt?
17-22: Version Input Description Enhancement
The updated description for theversioninput now provides clear guidance for prereleases versus stable releases as well as the option to override the dynamic version. Does this wording meet your communication goals for end users? wdyt?
45-50: Addition of Checkout CDK Repo Step
Adding the "Checkout CDK Repo" step (withfetch-depth: 0) ensures that the full Git history is available for accurate version detection. Would you consider this approach sufficient, or should we explore caching strategies to possibly improve build performance in the future? wdyt?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
docs/RELEASES.md (2)
24-30: Clarify and Enhance Pre-Release Steps InstructionsThe newly added pre-release steps clearly explain how to initiate the dynamic versioning process using Dunamai in the "Publish CDK" workflow. Would you consider adding a brief note or example of the expected version format (e.g.,
6.45.0.post15.dev0+76a92a93) or a direct link to the relevant Dunamai documentation to further clarify the process for users? wdyt?
72-73: Clarify Placeholder Replacement for Python Connector TestingThe instructions for updating the lockfile and running connector tests via Poetry are clear. However, would it be beneficial to explicitly remind users to replace the
<name>placeholder with the actual connector name to avoid any potential confusion? wdyt?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/RELEASES.md(2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/RELEASES.md
[grammar] ~70-~70: Did you mean the noun “publishing”?
Context: ...ng Low-Code Python connectors Once the publish pipeline has completed, set the version...
(PREPOSITION_VERB)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Check: 'source-pokeapi' (skip=false)
- GitHub Check: Check: 'source-amplitude' (skip=false)
- GitHub Check: Check: 'source-shopify' (skip=false)
- GitHub Check: Check: 'source-hardcoded-records' (skip=false)
- GitHub Check: Pytest (All, Python 3.11, Ubuntu)
- GitHub Check: Pytest (All, Python 3.10, Ubuntu)
- GitHub Check: SDM Docker Image Build
- GitHub Check: Pytest (Fast)
Aldo Gonzalez (aldogonzalez8)
left a comment
There was a problem hiding this comment.
Approved
168c1ab
into
main
Use Dunamai (https://dunamai.readthedocs.io) to calculate pre-release version when none is provided.
Now, with this change, you can just leave the version number blank when executing a manual pre-release publish workflow. The CI job will auto-calculate a version number for you so there's no need to create a unique one yourself.
6.45.0.post15.dev0+76a92a93To test locally, you can run this command:
Sample CI execution here: https://github.com/airbytehq/airbyte-python-cdk/actions/runs/14368872080/job/40287903080?pr=469
Summary by CodeRabbit