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: Consider missing optional input/output artifacts with same name #3029

Merged
merged 2 commits into from
May 13, 2020

Conversation

simster7
Copy link
Member

Fixes #1750

@simster7 simster7 requested a review from alexec May 13, 2020 21:32
@simster7 simster7 changed the title fix: Consider eponymous missing optional input/output artifacts fix: Consider missing optional input/output artifacts with same name May 13, 2020
@@ -0,0 +1,33 @@
apiVersion: argoproj.io/v1alpha1
Copy link
Contributor

Choose a reason for hiding this comment

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

move to ../testdata

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved!

artifacts:
- name: file
path: /tmp/file1.txt
optional: true
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure optional actually works - can you run without it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Running without optional: true fails the validation as expected:

$ argo submit art.yaml
2020/05/13 14:56:52 Failed to submit workflow: templates.plan.steps[0].write templates.file inputs.artifacts.file was not supplied

Copy link
Contributor

@alexec alexec left a comment

Choose a reason for hiding this comment

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

Conditional approval - can you check the execution time of the e2e tests and make sure they have increased significantly?

@sonarcloud
Copy link

sonarcloud bot commented May 13, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

80.0% 80.0% Coverage
0.0% 0.0% Duplication

@alexec
Copy link
Contributor

alexec commented May 13, 2020

Review alongside #3019.

@simster7
Copy link
Member Author

Conditional approval - can you check the execution time of the e2e tests and make sure they have increased significantly?

Assuming you meant "make sure that they haven't increased significantly". If so, the test only adds around 8-10 seconds. I think this is good enough.

Review alongside #3019.

I took a look and it looks orthogonal.

@simster7 simster7 merged commit 3d216ae into argoproj:master May 13, 2020
@simster7
Copy link
Member Author

Back-ported to v2.8.1

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.

Output artifact in same location as optional input artifact is not saved
2 participants