Skip to content
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: Make version bumping script more robust #2689

Merged
merged 4 commits into from
May 16, 2022

Conversation

achals
Copy link
Member

@achals achals commented May 13, 2022

Signed-off-by: Achal Shah [email protected]

Running locally is smooth:

$ python ./infra/scripts/release/bump_file_versions.py 0.21.0 0.22.0
Updated 12 files with new version 0.22.0
[13/05/22 3:10:09] (python-3.8.12) ~/tecton/feast   achal/update-version-bump-script ● 
$ git st
On branch achal/update-version-bump-script
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:   infra/charts/feast-python-server/Chart.yaml
	modified:   infra/charts/feast-python-server/README.md
	modified:   infra/charts/feast/Chart.yaml
	modified:   infra/charts/feast/README.md
	modified:   infra/charts/feast/charts/feature-server/Chart.yaml
	modified:   infra/charts/feast/charts/feature-server/README.md
	modified:   infra/charts/feast/charts/feature-server/values.yaml
	modified:   infra/charts/feast/charts/transformation-service/Chart.yaml
	modified:   infra/charts/feast/charts/transformation-service/README.md
	modified:   infra/charts/feast/charts/transformation-service/values.yaml
	modified:   infra/charts/feast/requirements.yaml
	modified:   infra/scripts/release/bump_file_versions.py
	modified:   java/pom.xml

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

raise SystemExit(f"Could not find {VERSIONS_TO_BUMP} versions of {current_version} in {path_to_file_list}")

print(f"Found {versions_in_files} occurrences of {current_version} in files to bump {path_to_file_list}")
# The current version should be 0.18.0 or 0.19.0 or 0.20.0 etc
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't think we need 0.18.0 or 0.19.0 right? Also after version bump we're at 0.21?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just as an example, but I'll update.

with open(repo_root.joinpath(file_path), "r") as f:
file_contents = f.readlines()
for line in lines:
assert current_version in file_contents[int(line) - 1], f"File {file_path} line {line} didn't containe " \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: "contain"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops fixed

@codecov-commenter
Copy link

codecov-commenter commented May 13, 2022

Codecov Report

Merging #2689 (0877fc4) into master (40563ca) will decrease coverage by 22.45%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #2689       +/-   ##
===========================================
- Coverage   80.98%   58.53%   -22.46%     
===========================================
  Files         166      166               
  Lines       13888    13832       -56     
===========================================
- Hits        11247     8096     -3151     
- Misses       2641     5736     +3095     
Flag Coverage Δ
integrationtests ?
unittests 58.53% <ø> (-0.14%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../integration/online_store/test_online_retrieval.py 17.39% <0.00%> (-82.61%) ⬇️
sdk/python/tests/utils/online_read_write_test.py 20.68% <0.00%> (-79.32%) ⬇️
.../integration/online_store/test_universal_online.py 17.64% <0.00%> (-77.78%) ⬇️
...fline_store/test_universal_historical_retrieval.py 24.17% <0.00%> (-75.83%) ⬇️
...ests/integration/e2e/test_python_feature_server.py 28.35% <0.00%> (-71.65%) ⬇️
sdk/python/feast/wait.py 23.52% <0.00%> (-70.59%) ⬇️
...gration/registration/test_feature_service_apply.py 31.25% <0.00%> (-68.75%) ⬇️
sdk/python/feast/infra/online_stores/redis.py 27.04% <0.00%> (-68.02%) ⬇️
...dk/python/tests/integration/e2e/test_validation.py 32.72% <0.00%> (-67.28%) ⬇️
sdk/python/tests/integration/e2e/test_usage_e2e.py 33.87% <0.00%> (-66.13%) ⬇️
... and 74 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 40563ca...0877fc4. Read the comment docs.

Signed-off-by: Achal Shah <[email protected]>
@achals achals self-assigned this May 16, 2022
Copy link
Collaborator

@adchia adchia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: achals, adchia

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@achals achals merged commit 1958df0 into feast-dev:master May 16, 2022
@achals achals deleted the achal/update-version-bump-script branch May 16, 2022 17:00
adchia pushed a commit that referenced this pull request May 16, 2022
* ci: Make version bumping script more robust

Signed-off-by: Achal Shah <[email protected]>

* bump dockerfiles too

Signed-off-by: Achal Shah <[email protected]>

* fix build-docker-image

Signed-off-by: Achal Shah <[email protected]>

* use buildx instead of build in makefile

Signed-off-by: Achal Shah <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants