-
Notifications
You must be signed in to change notification settings - Fork 686
[RFC-0012] Add command flux export source external
#5583
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
Conversation
c4851aa to
2709794
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most interesting parts of an ExternalArtifact are in .status 🤔
2709794 to
4f349ea
Compare
|
@matheuscscp - updated to I agree, do you think I should change export to also export the status? It is a bit of a different pattern then I have seen elsewhere, but happy to do it! |
Maybe only for ExternalArtifact... @stefanprodan Thoughts? |
4f349ea to
8074402
Compare
|
On a second thought, I'm not quite sure if exporting One could also end up thinking: "Well, @stefanprodan I guess this is what we discussed before, right? |
|
The export functions shouldn't print the status. In the case of EA, export is useful when the generation and reconciliation is decoupled, e.g. there is a dedicated controller that watcher external artifacts and sets their status. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dgunzy could you please add tests for this.
Signed-off-by: Daniel Guns <[email protected]>
8074402 to
cdc37c3
Compare
|
@stefanprodan @matheuscscp - I added a test in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks @dgunzy
Part of: #5504
[RFC-0012] Add command flux export source external
@stefanprodan - I added this, one thing I am not sure about is there is no
SecretRefso I am just returningnillet me know if there is a better way to go about this.