mgr/orch: increase readability for yaml representation#35537
Merged
sebastian-philipp merged 8 commits intoceph:masterfrom Jun 25, 2020
Merged
mgr/orch: increase readability for yaml representation#35537sebastian-philipp merged 8 commits intoceph:masterfrom
sebastian-philipp merged 8 commits intoceph:masterfrom
Conversation
13 tasks
Contributor
Author
|
kshtsk
reviewed
Jun 15, 2020
kshtsk
reviewed
Jun 15, 2020
kshtsk
reviewed
Jun 15, 2020
kshtsk
reviewed
Jun 15, 2020
kshtsk
reviewed
Jun 15, 2020
| out['version'] = self.version | ||
| out['status'] = self.status | ||
| out['status_desc'] = self.status_desc | ||
|
|
Contributor
There was a problem hiding this comment.
maybe this form will be syntactically sweeter for you:
out = OrderedDict([
('hostname', self.hostname),
('container_id', self.container_id),
('container_image_id', self.container_image_id),
('container_image_name', self.container_image_name),
('daemon_id', self.daemon_id),
('daemon_type', self.daemon_type),
('version', self.version),
('status', self.status),
('status_desc', self.status_desc),
])
kshtsk
reviewed
Jun 15, 2020
Comment on lines
+1398
to
+1344
| empty = [k for k, v in out.items() if v is None] | ||
| for e in empty: | ||
| del out[e] |
Contributor
There was a problem hiding this comment.
That's confusing, why not just use regular for?
for k,v in out.items():
if not v:
del out[k]
Contributor
There was a problem hiding this comment.
No, as it will modify dictionary size during iteration.
Contributor
There was a problem hiding this comment.
No, as it will modify dictionary size during iteration.
It should not be a problem, however, if you unsure, you can use only keys() and construct a list from it:
for k in list(out.keys()):
if not out[k]:
del out[k]
Contributor
Author
There was a problem hiding this comment.
yeah, that's what I'm doing here.
varshar16
reviewed
Jun 15, 2020
varshar16
reviewed
Jun 15, 2020
kshtsk
reviewed
Jun 15, 2020
varshar16
reviewed
Jun 16, 2020
ba6f132 to
61032bb
Compare
Contributor
Author
Contributor
Author
Contributor
|
jenkins test make check |
varshar16
reviewed
Jun 25, 2020
varshar16
reviewed
Jun 25, 2020
varshar16
reviewed
Jun 25, 2020
varshar16
reviewed
Jun 25, 2020
Contributor
Author
mind if I merge this as it is? otherwise I'll have to do another PR validation run for it. |
varshar16
reviewed
Jun 25, 2020
varshar16
reviewed
Jun 25, 2020
Contributor
After you clarify on this change here: |
61032bb to
6515c3e
Compare
* Add test for new yaml representation Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
* Changes: An empty OSD Spec is now invalid. * OSDSpec.validate() now fails, if service-id is empty Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
Signed-off-by: Sebastian Wagner <sebastian.wagner@suse.com>
6515c3e to
5150e95
Compare
Devp00l
reviewed
Jun 25, 2020
| # Please do not modify those JSON values. | ||
|
|
||
| spec = ServiceSpec.from_json(spec_json) | ||
| # just some verification that we can sill read old octopus specs |
There was a problem hiding this comment.
Found a typo
Suggested change
| # just some verification that we can sill read old octopus specs | |
| # just some verification that we can still read old octopus specs |
Contributor
Author
Contributor
Author
|
jenkins test dashboard backend |
mgfritch
approved these changes
Jun 25, 2020
varshar16
approved these changes
Jun 25, 2020
Contributor
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
ceph orch ls --format=yamlbefore:after:
Fixes: https://tracker.ceph.com/issues/45321
Checklist
Show available Jenkins commands
jenkins retest this pleasejenkins test classic perfjenkins test crimson perfjenkins test signedjenkins test make checkjenkins test make check arm64jenkins test submodulesjenkins test dashboardjenkins test dashboard backendjenkins test docsjenkins render docsjenkins test ceph-volume alljenkins test ceph-volume tox