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

LBaaS v2 l7 policy support - part 5: update l7policy #905

Merged
merged 2 commits into from Apr 10, 2018

Conversation

@coveralls
Copy link

coveralls commented Apr 8, 2018

Coverage Status

Coverage increased (+0.03%) to 77.39% when pulling c509aa9 on lingxiankong:update-l7policy into a4e7a3c on gophercloud:master.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Apr 8, 2018

Build failed.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Apr 8, 2018

Build succeeded.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Apr 8, 2018

Build succeeded.

@lingxiankong
Copy link
Contributor Author

@jtopjian @jrperritt hi, update l7 policy is ready to review :-)

Copy link
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

@lingxiankong Overall this looks good. I have two questions about the fields.

// operation.
type UpdateOpts struct {
// Name of the L7 policy.
Name string `json:"name,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to remove a name? If so, this should be *string. That will allow a value of "" to be sent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a valid question, @jtopjian I tested in my octavia environment, an empty string for both name and description are allowed to be specified to update l7 policy, i will update the PR

Position int32 `json:"position,omitempty"`

// A human-readable description for the resource.
Description string `json:"description,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to remove a description? If so, this should be *string.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Apr 9, 2018

Build failed.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Apr 9, 2018

Build succeeded.

@lingxiankong
Copy link
Contributor Author

@jtopjian PR updated

@jtopjian
Copy link
Contributor

LGTM - thank you :)

The example in doc.go needs to be updated to reflect the pointer-to-string, but I'll take care of that.

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.

None yet

3 participants