Replace sed with ruamel.yaml for packages.yml version bumps#2148
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
👋 @devin-ai-integration[bot] |
|
Warning Review limit reached
More reviews will be available in 11 minutes and 38 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds a Python script to update the elementary package version in Changes
Sequence Diagram(s)sequenceDiagram
participant GH as "GitHub Actions"
participant Runner as "Action runner (env)"
participant Python as "bump_packages_version.py"
participant YAML as "packages.yml (repo)"
participant Git as "git commit/push"
GH->>Runner: start workflow
Runner->>Runner: Determine package version\n(write to GITHUB_OUTPUT)
Runner->>Python: export PKG_VERSION\nrun script
Python->>YAML: read `packages.yml`
Python->>YAML: update/replace elementary entry\nappend helper comments
Python-->>Runner: exit 0 / success message
Runner->>Git: commit & push updated `packages.yml`
Git-->>GH: push confirmed
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/bump_packages_version.py:
- Around line 19-20: The script currently uses PyYAML's yaml.safe_load() and
yaml.dump(), which strip comments from PACKAGES_FILE during the
read/modify/write round-trip; replace this with a round-trip YAML API that
preserves comments (e.g., ruamel.yaml's YAML() loader/dumper) so comments and
formatting in PACKAGES_FILE are retained when reading into the variable
currently named data and when writing back (the code paths that call yaml.dump
at the end of the script). Update imports and replace yaml.safe_load(...) /
yaml.dump(...) usage with the ruamel YAML().load(...) and YAML().dump(...)
workflow (or an equivalent round-trip-preserving YAML library) while ensuring
the same data modifications still target the in-memory variable (data) and are
written back to the same file.
In @.github/workflows/bump-version.yml:
- Around line 70-77: Update the "Determine package version" step (id:
pkg-version) to fail when a user-supplied dbt-package-version override is
present but validation produced an empty validated-dbt-package-version: check
whether the override (the dbt-package-version
input/needs.validate-version.outputs.requested-dbt-package-version or
equivalent) is non-empty and the
needs.validate-version.outputs.validated-dbt-package-version is empty, and if so
echo an error and exit non‑zero instead of falling back to
default-dbt-package-version; otherwise continue to set version to the validated
value or default as before.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 122bfa05-a351-4733-a09d-c2486fa51890
📒 Files selected for processing (2)
.github/scripts/bump_packages_version.py.github/workflows/bump-version.yml
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/bump-version.yml (1)
68-69: Pin PyYAML to a vetted version for deterministic releases.Installing
pyyamlwithout a version can make this release workflow non-reproducible and brittle over time.Proposed change
- - name: Install PyYAML - run: pip install pyyaml + - name: Install PyYAML + run: python -m pip install "PyYAML==<vetted-version>"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/bump-version.yml around lines 68 - 69, Update the "Install PyYAML" workflow step to install a specific, vetted PyYAML release instead of the unpinned package; change the pip command in the Install PyYAML step to pin to a known good version (for example pyyaml==6.0) so the release workflow is deterministic and reproducible.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/bump-version.yml:
- Around line 68-69: Update the "Install PyYAML" workflow step to install a
specific, vetted PyYAML release instead of the unpinned package; change the pip
command in the Install PyYAML step to pin to a known good version (for example
pyyaml==6.0) so the release workflow is deterministic and reproducible.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97eb3bf2-b98f-4c4b-b605-4cf25c71eb0a
📒 Files selected for processing (1)
.github/workflows/bump-version.yml
themavik
left a comment
There was a problem hiding this comment.
Reviewed the changes — the implementation looks correct and addresses the reported issue well.
|
This pull request is stale because it has been open for too long with no activity. |
|
Suggestion: use The YAML-parsing direction is right, but Swap import os
import sys
from ruamel.yaml import YAML
from ruamel.yaml.comments import CommentedMap
PACKAGES_FILE = "./elementary/monitor/dbt_project/packages.yml"
def bump_packages_version(version: str) -> None:
yaml = YAML()
yaml.preserve_quotes = True
yaml.indent(mapping=2, sequence=4, offset=2) # match the file's existing style
with open(PACKAGES_FILE) as f:
data = yaml.load(f)
packages = data.get("packages") or []
elementary_found = False
for i, pkg in enumerate(packages):
if "git" in pkg and "dbt-data-reliability" in pkg["git"]:
# Convert the active git-hash entry into a versioned package entry,
# carrying over any comment ruamel anchored to the old node's last key.
new_pkg = CommentedMap()
new_pkg["package"] = "elementary-data/elementary"
new_pkg["version"] = version
for _key, comment in pkg.ca.items.items():
new_pkg.ca.items["version"] = comment
packages[i] = new_pkg
elementary_found = True
break
if pkg.get("package") == "elementary-data/elementary":
pkg["version"] = version
elementary_found = True
break
if not elementary_found:
print(
"::error::Could not find elementary-data/elementary or "
"dbt-data-reliability entry in packages.yml"
)
sys.exit(1)
with open(PACKAGES_FILE, "w") as f:
yaml.dump(data, f)
print(f"Updated packages.yml to version {version}")
if __name__ == "__main__":
version = os.environ.get("PKG_VERSION", "")
if not version:
print("::error::PKG_VERSION environment variable is not set")
sys.exit(1)
bump_packages_version(version)A few other things to fold in while rebasing (the branch is currently conflicting with master):
|
|
❌ Cannot revive Devin session - the session is too old. Please start a new session instead. |
The sed-based version replacement in bump-version.yml had two issues: 1. It only matched 'version: X.Y.Z' patterns, missing the case where master has a git hash reference for dbt-data-reliability 2. It also matched commented-out version lines Replace the sed command for packages.yml with a Python script using ruamel.yaml (round-trip YAML parser) that preserves comments, quotes, flow-style, and indentation. The script handles both cases: - Updates an existing package version reference (one-line diff) - Replaces a git hash reference with a proper package version Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
e69aaab to
933779a
Compare
|
@haritamar Done — rebased on master and switched to The script now produces a true one-line diff for normal releases and preserves all comments, quotes, flow-style sequences ( Other changes from the rebase:
|
Co-Authored-By: Itamar Hartstein <haritamar@gmail.com>
Summary
The
sed-based version replacement inbump-version.ymlonly matchedversion: X.Y.Zpatterns, so whenmasterhad a git hash reference (e.g.- git: ... dbt-data-reliability.git+revision: <hash>), the version bump silently did nothing. It also matched commented-out lines.This PR replaces the
sedcommand forpackages.ymlwith a Python script usingruamel.yaml(round-trip YAML parser). Unlike the previous PyYAML-based approach,ruamel.yamlpreserves comments, quotes, flow-style sequences, and indentation — producing a true one-line diff on a normal release:The script handles both paths:
versionin-place on the existingelementary-data/elementaryentry- git: .../dbt-data-reliability.git+revision: ...entry with a- package: elementary-data/elementary+version: ...entry, carrying over trailing commentsruamel.yamlis pinned to==0.18.10for deterministic builds. Thedbt deps --lockstep from master and the existingsedforpyproject.tomlare preserved as-is.Link to Devin session: https://app.devin.ai/sessions/0375709f72a94fdda7ce6654be1bb8b1
Requested by: @haritamar