Add error fields to cluster/repo, shell output#200
Add error fields to cluster/repo, shell output#200merenbach merged 5 commits intoargoproj:masterfrom merenbach:add-repo-cluster-error-field
Conversation
cmd/argocd/commands/repo.go
Outdated
There was a problem hiding this comment.
There was a problem hiding this comment.
@alexmt good catch. I should have run GoMetaLinter before committing. Mea culpa.
jessesuen
left a comment
jessesuen
left a comment
There was a problem hiding this comment.
On second thought, can we use the term Message instead of Error? This follows what we do everywhere else and doesn't limit us to using the field to just errors.
|
@jessesuen have renamed Error => Message |
There was a problem hiding this comment.
How come the json annotation is -? Shouldn't it be message?
There was a problem hiding this comment.
Maybe that is my misunderstanding about how this is supposed to work. Where all these other fields seem to me to be permanent type records about configuration, the message field I figured shouldn't be committed anywhere since it's not something that would be restored when creating a cluster/repo, say when importing an existing config.
Put another way, I've been operating under the impression that this field is a scratchpad for the running process to fill out, but not something that will be committed into K8s or any sort of long-term storage.
ETA: @jessesuen I can definitely rename the field if that more closely aligns with the spec you had in mind.
There was a problem hiding this comment.
I do agree that message is not something a user should ever set. But it is something that needs to be persisted. The idea is that when we detect issues with a cluster or repo, we would denote it in the object and save it. Then at any given time, a argocd repo list or argocd cluster list will display the value. We can always ignore whatever value we receive in the Create/Update payload.
There was a problem hiding this comment.
Just to add -- this is typically something that would typically belong under a status field of an object for normal k8s objects. But since this is just lightweight config objects, we don't have spec/status so everything is just going into a flatter definition.
|
Since we don't have an implementation for filling out Message. Let's hold off on merging this until after we cut 0.4. I want to avoid protobuf migrations until we actually start using the field. |
…roj#200) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from v1.1.1 to v1.2.0. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/master/CHANGELOG.md) - [Commits](codecov/codecov-action@v1.1.1...a92c414) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
3 participants
Closes #169
@jessesuen please let me know if the scope of issue #169 includes populating the error fields. ;-)