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

Workflow to update Dapr. #249

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

artursouza
Copy link
Member

Description

Workflow to update Dapr runtime version in longhaul environment.

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: N/A

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: Artur Souza <[email protected]>
Copy link
Contributor

@elena-kolevska elena-kolevska left a comment

Choose a reason for hiding this comment

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

Just one suggestion.
Let me know if you'd like me to implement it.
Also, we should do the same for cli releases.

run: |
sudo apt-get update
sudo apt-get install pcre2-utils
pip install packaging
Copy link
Contributor

@elena-kolevska elena-kolevska Jan 28, 2025

Choose a reason for hiding this comment

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

Suggested change
pip install packaging
pip install semver

I suggest we move the compare_versions.py file from the dapr repo to this repo and update it so it uses the semver package instead of the packaging one that uses the PEP440 standard for version validation and comparison.

Comment on lines 50 to 58
# Thanks to https://ihateregex.io/expr/semver/
SEMVER_REGEX='^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$'
REL_VERSION=`echo "${{ inputs.rel_version }}" | sed -r 's/^[vV]?([0-9].+)$/\1/'`
if [ `echo $REL_VERSION | pcre2grep "$SEMVER_REGEX"` ]; then
echo "$REL_VERSION is a valid semantic version."
else
echo "$REL_VERSION is not a valid semantic version."
exit 1
fi
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# Thanks to https://ihateregex.io/expr/semver/
SEMVER_REGEX='^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$'
REL_VERSION=`echo "${{ inputs.rel_version }}" | sed -r 's/^[vV]?([0-9].+)$/\1/'`
if [ `echo $REL_VERSION | pcre2grep "$SEMVER_REGEX"` ]; then
echo "$REL_VERSION is a valid semantic version."
else
echo "$REL_VERSION is not a valid semantic version."
exit 1
fi
# Thanks to https://ihateregex.io/expr/semver/
SEMVER_REGEX='^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$'
REL_VERSION=`echo "${{ inputs.rel_version }}" | sed -r 's/^[vV]?([0-9].+)$/\1/'`
if [ `echo $REL_VERSION | pcre2grep "$SEMVER_REGEX"` ]; then
echo "$REL_VERSION is a valid semantic version."
else
echo "$REL_VERSION is not a valid semantic version."
exit 1
fi

If we accept what I suggested in the comment above, we can also remove the regex check, because it will be covered in the compare_versions.py check below.

Copy link
Member

@mikeee mikeee left a comment

Choose a reason for hiding this comment

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

I'd like to see a dry-run (implemented in the workflow)

.github/workflows/version-update-cli.yml Outdated Show resolved Hide resolved
.github/workflows/version-update-dapr.yml Outdated Show resolved Hide resolved
Co-authored-by: Mike Nguyen <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
@elena-kolevska
Copy link
Contributor

I'd like to see a dry-run (implemented in the workflow)

Something like what would be the output in the config files, without actually writing and committing them?
I don't know how much additional value that would bring since we're passing the versions as inputs, but we can definitely do it. Should I add another input called dry-run?

Copy link
Member

@mikeee mikeee left a comment

Choose a reason for hiding this comment

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

Sorry - looks like my indentations were off

.github/workflows/version-update-dapr.yml Outdated Show resolved Hide resolved
.github/workflows/version-update-cli.yml Outdated Show resolved Hide resolved
Co-authored-by: Mike Nguyen <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
.github/workflows/version-update-cli.yml Outdated Show resolved Hide resolved
.github/workflows/version-update-dapr.yml Outdated Show resolved Hide resolved
Co-authored-by: Mike Nguyen <[email protected]>
Signed-off-by: Elena Kolevska <[email protected]>
Copy link
Member

@mikeee mikeee left a comment

Choose a reason for hiding this comment

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

These steps don't appear to be used

Comment on lines +51 to +52
- name: Parse release version and set REL_VERSION and LATEST_RELEASE
run: python .dapr_cli/.github/scripts/get_release_version.py ${{ github.event_name }}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Parse release version and set REL_VERSION and LATEST_RELEASE
run: python .dapr_cli/.github/scripts/get_release_version.py ${{ github.event_name }}

Comment on lines +50 to +51
- name: Parse release version and set REL_VERSION and LATEST_RELEASE
run: python .dapr_repo/.github/scripts/get_release_version.py ${{ github.event_name }}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- name: Parse release version and set REL_VERSION and LATEST_RELEASE
run: python .dapr_repo/.github/scripts/get_release_version.py ${{ github.event_name }}

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.

3 participants