Skip to content

Decouple client release from Delphi release #1465

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

Merged
merged 9 commits into from
Jun 25, 2024

Conversation

rzats
Copy link
Contributor

@rzats rzats commented Jun 3, 2024

Closes #1296.

Summary:

Semi-detaches the versioning of the Python client and delphi-epidata.

This means that a new version of the Python client is only released when changes are made to the client file - when this is the case, however, it is synced to the current delphi-epidata version.

For further details, see #1296 (comment).

Prerequisites:

  • Unless it is a documentation hotfix it should be merged against the dev branch
  • Branch is up-to-date with the branch to be merged with, i.e. dev
  • Build is successful
  • Code is cleaned up and formatted

run: echo "sha=$(git rev-parse origin/main)" >> $GITHUB_OUTPUT
- name: Get Python client file changes
id: changed-py
uses: tj-actions/changed-files@v44
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we use dorny/paths-filter here (and also in release-helper.yml) to match the usage in the covidcast-indicators repo? fewer dependencies and reinvented wheels make for easier support and maintenance.
https://github.com/cmu-delphi/covidcast-indicators/blob/a6ea00355d79a47cc09db8090f6e5121c718de0f/.github/workflows/create-release.yml#L32-L40
https://github.com/cmu-delphi/covidcast-indicators/blob/a6ea00355d79a47cc09db8090f6e5121c718de0f/.github/workflows/publish-release.yml#L31-L41

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can give it a go, it should support the same features as changed-files.

@dshemetov
Copy link
Contributor

Pushed a merge from dev and resolved conflicts with #1470.

dshemetov and others added 3 commits June 7, 2024 11:53
* add comments
* revert bumpversion configs

Co-authored-by: george <[email protected]>
fix(pyclient ci): validate version arg in create-release
@rzats
Copy link
Contributor Author

rzats commented Jun 21, 2024

@melange396 @dshemetov this is ready once again! Here's an updated rundown of my CI tests as well.

For testing purposes, I've forked the delphi-epidata repository, then added the commit rzats@0e9500f to it. This is a version of this PR which enables create-release and release-helper within a fork, but also stubs out various destructive steps in the GitHub Action - like actually creating a PyPi release, for instance.

I then ran several mock releases within the same repository:

4.1.24

rzats#1

4.1.25

rzats#3

4.1.26

rzats#5

@rzats rzats requested a review from melange396 June 21, 2024 15:48
dshemetov
dshemetov previously approved these changes Jun 21, 2024
Copy link
Contributor

@dshemetov dshemetov left a comment

Choose a reason for hiding this comment

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

So I looked over your fork and its associated PRs. The overall behavior looks to be consistent with what you describe and what you and George agreed on in #1296 (namely, Python client stays loosely coupled - it doesn't update if there are no changes to delphi_epidata.py or contents of .../packaging/pypi/, but then "catches up" to the main repo version when there is a change). No further notes from me.

Copy link

@rzats
Copy link
Contributor Author

rzats commented Jun 25, 2024

@melange396 @dshemetov I've updated the PR again! Also reran the tests and updated the links here: #1465 (comment)

@rzats rzats requested review from dshemetov and melange396 June 25, 2024 13:43
@melange396
Copy link
Collaborator

It looks like you tested this with just "patch" as the argument to "Create Release"... Did you try with "major" (or "minor") and an explicit version to make sure those do the right thing too?

@rzats
Copy link
Contributor Author

rzats commented Jun 25, 2024

@melange396 I ran some tests for these just now and everything's looking OK:

minor: rzats#7 4.1.264.2.0
major: rzats#9 4.2.05.0.0
custom: rzats#11 5.0.06.7.8; also changes the Python client to make sure it jumps to the right version

@melange396 melange396 merged commit 10a0a6f into dev Jun 25, 2024
7 checks passed
@melange396 melange396 deleted the rzatserkovnyi/client-release-decouple branch June 25, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python client -- issues with version coupling and automatic releases
3 participants