Repository navigation
dev: Allow new release version to be automatically updated - #26007
nuno-faria wants to merge 2 commits into
Conversation
| crates = { | ||
| 'datafusion-common': 'datafusion/common/Cargo.toml', | ||
| 'datafusion-common-runtime': 'datafusion/common-runtime/Cargo.toml', | ||
| 'datafusion': 'datafusion/core/Cargo.toml', | ||
| 'datafusion-execution': 'datafusion/execution/Cargo.toml', | ||
| 'datafusion-expr': 'datafusion/expr/Cargo.toml', | ||
| 'datafusion-ffi': 'datafusion/ffi/Cargo.toml', | ||
| 'datafusion-functions': 'datafusion/functions/Cargo.toml', | ||
| 'datafusion-functions-aggregate': 'datafusion/functions-aggregate/Cargo.toml', | ||
| 'datafusion-functions-nested': 'datafusion/functions-nested/Cargo.toml', | ||
| 'datafusion-optimizer': 'datafusion/optimizer/Cargo.toml', | ||
| 'datafusion-physical-expr': 'datafusion/physical-expr/Cargo.toml', | ||
| 'datafusion-physical-expr-common': 'datafusion/physical-expr-common/Cargo.toml', | ||
| 'datafusion-physical-plan': 'datafusion/physical-plan/Cargo.toml', | ||
| 'datafusion-proto': 'datafusion/proto/Cargo.toml', | ||
| 'datafusion-sql': 'datafusion/sql/Cargo.toml', | ||
| 'datafusion-sqllogictest': 'datafusion/sqllogictest/Cargo.toml', | ||
| 'datafusion-substrait': 'datafusion/substrait/Cargo.toml', | ||
| 'datafusion-wasmtest': 'datafusion/wasmtest/Cargo.toml', | ||
| 'datafusion-benchmarks': 'benchmarks/Cargo.toml', | ||
| 'datafusion-cli': 'datafusion-cli/Cargo.toml', | ||
| 'datafusion-examples': 'datafusion-examples/Cargo.toml', | ||
| } |
There was a problem hiding this comment.
This was not catching everything, and is also not needed since all crates start with "datafusion".
| def update_datafusion_version(cargo_toml: str, new_version: str): | ||
| print(f'updating {cargo_toml}') | ||
| with open(cargo_toml) as f: | ||
| data = f.read() | ||
|
|
||
| doc = tomlkit.parse(data) | ||
| pkg = doc.get('package') | ||
| if 'workspace' not in pkg['version']: | ||
| pkg['version'] = new_version | ||
|
|
||
| with open(cargo_toml, 'w') as f: | ||
| f.write(tomlkit.dumps(doc)) | ||
|
|
||
| def update_downstream_versions(cargo_toml: str, new_version: str): | ||
| with open(cargo_toml) as f: | ||
| data = f.read() | ||
|
|
||
| doc = tomlkit.parse(data) | ||
|
|
||
| for crate in crates.keys(): | ||
| df_dep = doc.get('dependencies', {}).get(crate) | ||
| # skip crates that pin datafusion using git hash | ||
| if df_dep is not None and df_dep.get('version') is not None: | ||
| print(f'updating {crate} dependency in {cargo_toml}') | ||
| df_dep['version'] = new_version | ||
|
|
||
| df_dep = doc.get('dev-dependencies', {}).get(crate) | ||
| if df_dep is not None and df_dep.get('version') is not None: | ||
| print(f'updating {crate} dev-dependency in {cargo_toml}') | ||
| df_dep['version'] = new_version |
There was a problem hiding this comment.
The subcrates use the workspace version directly, so this is not needed anymore.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #26007 +/- ##
==========================================
- Coverage 82.64% 82.64% -0.01%
==========================================
Files 1147 1147
Lines 445502 445502
Branches 445502 445502
==========================================
- Hits 368179 368174 -5
- Misses 54969 54971 +2
- Partials 22354 22357 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
VVKot
left a comment
There was a problem hiding this comment.
Thanks for your contribution!
One more thing I've noticed - docs/source/user-guide/configs.md now gets changed by the script - I believe incorrectly:
-| datafusion.execution.parquet.created_by | datafusion version 55.1.0 | (writing) Sets "created by" property
|
+| datafusion.execution.parquet.created_by | datafusion version 56.0.0 | (writing) Sets "created by" property
| Within the user documentation there are references to the current version number. | ||
| Update these to the current version. At the time of this writing we need to manually | ||
| update the following files | ||
| This updates the DataFusion version across all files, including documentation. The only extra step required is to update the `Cargo.lock` file, using the following command: |
There was a problem hiding this comment.
It does sound like the manual cargo check is actually needed? For me, the script is updating Cargo.lock automatically, and cargo check -p datafusion produces no new diff
There was a problem hiding this comment.
The Cargo.lock is not updated by the script. Are you maybe running rust-analyzer in the background (e.g., in vscode)? I tested again and these are the files that changed for me:
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: Cargo.toml
modified: ci/scripts/release_version_labeler.js
modified: docs/source/download.md
modified: docs/source/user-guide/configs.md
modified: docs/source/user-guide/crate-configuration.md
modified: docs/source/user-guide/example-usage.md
There was a problem hiding this comment.
Yep, that's exactly what it was -- background rust-analyzer process updating the Cargo.lock!
| for crate in crates.keys(): | ||
| df_dep = doc.get('workspace').get('dependencies', {}).get(crate) | ||
| for crate, df_dep in doc['workspace']['dependencies'].items(): | ||
| if not crate.startswith('datafusion'): |
There was a problem hiding this comment.
Per your above comment, this seems like a reasonable way to enumerate all crates now - but I don't know if that will always be the case.
Would love one of the committers to opine whether we can expect all crate names to start with datafusion
There was a problem hiding this comment.
I believe so, all DataFusion subcrates get updated when a new version is released, and they should always have the same prefix.
| update_docs("docs/source/download.md", new_version) | ||
| update_docs("docs/source/user-guide/example-usage.md", new_version) | ||
| update_docs("docs/source/user-guide/crate-configuration.md", new_version) | ||
| update_docs("docs/source/user-guide/configs.md", new_version) |
There was a problem hiding this comment.
see my top-level comment - update_docs is a bit too naive to properly update configs.md
There was a problem hiding this comment.
I'm not sure what the issue is, the diff looks ok to me.
There was a problem hiding this comment.
created_by seems to be the default from 55.1.0 but the script will keep updating it #26007 (review) , which is misleading
There was a problem hiding this comment.
I'm still not sure what the issue is, the created_by version in configs.md needs to be updated to the new one as well. The value should match CARGO_PKG_VERSION.
There was a problem hiding this comment.
Ah I see, my initial interpretation of this row in the table was
created_byproperty is set by default since 55.1.0
whereas it actually means
created_byproperty will be set to stringdatafusion version 55.1.0
All good here.
|
Thanks @VVKot for the review. |
Which issue does this PR close?
Rationale for this change
The release docs instructed to manually update the release version. This new version points to the script to do this automatically. Also updated that script since the previous version was not updating the version in all places.
What changes are included in this PR?
What is the testing strategy for this PR?
Manual testing.
Are there any user-facing changes?
No.