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

Fix compatibility with Argo Workflows 3.6 #1293

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

elliotgunton
Copy link
Collaborator

Pull Request Checklist

Description of PR
Updates auto-generated models to Argo 3.6. Note there are a few changes to Hera code which are technically breaking changes (but are very simple for users to change):

  • resources: Optional[ResourceRequirements] is now resources: Optional[VolumeResourceRequirements] for volumes. The attributes in the classes are the same except for claims is removed in VolumeResourceRequirements. Updated 4 examples that were using this
  • cluster_name has been removed in ObjectMeta so is also removed in the Workflow class. It was never used by the Argo server (and is explicitly noted as an "ignored" field in the field description in ObjectMeta).

The rest are changes in the Argo models (i.e. in src/hera/workflows/models/io/argoproj/workflow/v1alpha1.py), that will affect users using the models directly. i.e. They'll be (relatively1) stuck on the current Hera version until they upgrade Argo to 3.6 if they are using the following classes in Argo<3.6:

  1. The workaround would be to use ModelClass.construct as seen here.

* Manually update scripts/spec.py to remove Container remapping of `enum`
  field (it is now a string)

Signed-off-by: Elliot Gunton <[email protected]>
* NOTE THIS IS BACKWARDS INCOMPAT DUE TO TYPE CHANGES IN ARGO 3.6

Signed-off-by: Elliot Gunton <[email protected]>
* Also remove pyi files to fix lint

Signed-off-by: Elliot Gunton <[email protected]>
Signed-off-by: Elliot Gunton <[email protected]>
@elliotgunton elliotgunton added semver:minor A change requiring a minor version bump type:informational Provides information or notice to the community labels Dec 12, 2024
Copy link

codecov bot commented Dec 12, 2024

Codecov Report

Attention: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.

Project coverage is 86.3%. Comparing base (964f400) to head (f1b93b1).

Files with missing lines Patch % Lines
src/hera/workflows/volume.py 50.0% 2 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff          @@
##            main   #1293   +/-   ##
=====================================
  Coverage   86.2%   86.3%           
=====================================
  Files         60      60           
  Lines       4091    4095    +4     
  Branches     653     653           
=====================================
+ Hits        3530    3534    +4     
  Misses       391     391           
  Partials     170     170           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Manual changes required for #1215

---------

Signed-off-by: Elliot Gunton <[email protected]>
* Used by Container, ContainerNode and Script

---------

Signed-off-by: Elliot Gunton <[email protected]>
Signed-off-by: Elliot Gunton <[email protected]>
* Fixes `make test-on-cluster`, but unsure why the service account changed

Signed-off-by: Elliot Gunton <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:minor A change requiring a minor version bump type:informational Provides information or notice to the community
Projects
None yet
2 participants