WorkflowTemplate/ClusterWorkflowTemplate create_as_workflow #669
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
elliotgunton
added
type:enhancement
Codecov Report
@@ Coverage Diff @@
## main #669 +/- ##
=======================================
+ Coverage 74.5% 76.1% +1.6%
=======================================
Files 44 45 +1
Lines 3030 3088 +58
Branches 577 584 +7
=======================================
+ Hits 2259 2352 +93
+ Misses 597 566 -31
+ Partials 174 170 -4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
flaviuvadan
reviewed
Jun 5, 2023
flaviuvadan
reviewed
Jun 5, 2023
flaviuvadan
reviewed
Jun 5, 2023
flaviuvadan
reviewed
Jun 5, 2023
Open
elliotgunton
force-pushed
the
create-as-workflow
branch
from
ab6fba6
to
b5a304b
Compare
Merged
elliotgunton
added a commit
that referenced
this pull request
Jun 6, 2023
While working on top of #669, thought we could work towards better testing and because the patch coverage was so low I figured it would help. Separate PR to highlight changes but will merge down * Adds mocker `dev` dependency * Create test_unit folder, added `Workflow.create` test to demonstrate ideal usage * Add `cov-report=term-missing` to pytest `make test` target to encourage us to improve coverage - this will report a table back to us in terminal of coverage and missing lines --------- Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
elliotgunton
force-pushed
the
create-as-workflow
branch
from
09ce72e
to
547ad57
Compare
elliotgunton
added a commit
that referenced
this pull request
Jun 6, 2023
While working on top of #669, thought we could work towards better testing and because the patch coverage was so low I figured it would help. Separate PR to highlight changes but will merge down * Adds mocker `dev` dependency * Create test_unit folder, added `Workflow.create` test to demonstrate ideal usage * Add `cov-report=term-missing` to pytest `make test` target to encourage us to improve coverage - this will report a table back to us in terminal of coverage and missing lines --------- Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
elliotgunton
force-pushed
the
create-as-workflow
branch
4 times, most recently
from
b6bbc0f
to
87465ce
Compare
flaviuvadan
reviewed
Jun 8, 2023
flaviuvadan
reviewed
Jun 8, 2023
There was a problem hiding this comment.
Looks great @elliotgunton 🎖️ I left a question re: interfacing with the generated name option. Will approve after the discussion/potential change
elliotgunton
force-pushed
the
create-as-workflow
branch
3 times, most recently
from
91323ec
to
41af9fe
Compare
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
While working on top of #669, thought we could work towards better testing and because the patch coverage was so low I figured it would help. Separate PR to highlight changes but will merge down * Adds mocker `dev` dependency * Create test_unit folder, added `Workflow.create` test to demonstrate ideal usage * Add `cov-report=term-missing` to pytest `make test` target to encourage us to improve coverage - this will report a table back to us in terminal of coverage and missing lines --------- Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
* Remove pytest-mock usage (use unittest.mock) Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
Signed-off-by: Elliot Gunton <egunton@bloomberg.net>
elliotgunton
force-pushed
the
create-as-workflow
branch
from
5ee34c6
to
3e3c528
Compare
4 tasks
4 tasks
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist
run_as_workflowmethod #521Description of PR
Currently, the DevEx for a workflow template is either:
workflowTemplateRef,templateRefor via CLIsubmit --fromThis PR adds
create_as_workflowfunction (reflecting existing naming in theWorkflowclass rather thanrun_as)kindin place, then submits to Argo throughWorkflow.createwaitandpoll_interval(was hesitant to just usekwargs)ClusterWorkflowTemplateas a subclass ofWorkflowTemplateas it was missing, added toTWorkflowtypeWorkflowTemplate, as we need to call the rightWorkflowServicefunctionsbuildjust converts the pydantic model toClusterWorkflowTemplate.