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

Edit command #1564

Closed
wants to merge 2 commits into from
Closed

Conversation

robbrockbank
Copy link
Contributor

@robbrockbank robbrockbank commented Mar 10, 2017

Allows for calicoctl edit X, which opens resource X in an editor and applies changes on-save to the datastore.

@robbrockbank robbrockbank force-pushed the edit-command branch 2 times, most recently from dfb0916 to 19af050 Compare March 10, 2017 22:10
@robbrockbank
Copy link
Contributor Author

This was a prototype - that I don't think needs a whole lot to get it production ready. I'm leaving here for now so that if someone wants to take it on they can. There is a TODO list in the edit.go file.

@@ -102,6 +102,18 @@ func convertToSliceOfResources(loaded interface{}) []unversioned.Resource {
return r
}

func convertToSliceOfResourceObjects(resources []unversioned.Resource) ([]unversioned.ResourceObject, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the method convertToSliceOfResources should actually be convertToSliceOfResourceObjects ... I think that was it's intention - so shouldn't need this separate function.

// Assume that that the resources are kept in the same order
for i, _ := range original {
if original[i].String() != changed[i].String() {
return fmt.Errorf("Cannot change metadata fields: %s should match %s", changed[i].String(), original[i].String())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not quite true - you are allowed to change the labels which are a metadata field.

@@ -174,9 +169,20 @@ Description:

// Convert the file into a set of resources. If this succeeds
// then it passes validation, so exit this loop.
var editedObjs []unversioned.ResourceObject
var prevObjs []unversioned.ResourceObject
resources, err = resourcemgr.CreateResourcesFromBytes(b)
if err == nil {
Copy link
Contributor Author

@robbrockbank robbrockbank Jul 26, 2017

Choose a reason for hiding this comment

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

I think it would be easier to invert this logic to reduce the nesting.

Better I think now to check if err != nil to log a specific log and then continue the loop, eg.

if resources, err = resourcemgr.CreateResourcesFromBytes(b); err != nil {
    log
    continue
} else if editedObjs, err = convertToSliceOfResourceObjects(resources); err != nil {
    log
    continue
} else ...

}

break

@mgleung mgleung force-pushed the edit-command branch 2 times, most recently from 2af24ce to 186f974 Compare July 26, 2017 23:34
@mgleung
Copy link
Contributor

mgleung commented Jul 27, 2017

TODO:

@mgleung mgleung changed the title [WIP] Edit command Edit command Jul 28, 2017
@mgleung mgleung force-pushed the edit-command branch 3 times, most recently from b6b26a3 to 6dc39cc Compare July 28, 2017 22:36
@caseydavenport
Copy link
Member

This PR has gotten pretty out of date, and is quite old.

I'd love to see this feature added, but for now I'm going to close this. Shout if you'd like to pick it back up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants