Skip to content

feat: update Messaging v2 API to support _enable_encrypt_messages field#2016

Merged
jtopjian merged 1 commit intogophercloud:masterfrom
galuszkak:feature/messaging-update
Sep 11, 2020
Merged

feat: update Messaging v2 API to support _enable_encrypt_messages field#2016
jtopjian merged 1 commit intogophercloud:masterfrom
galuszkak:feature/messaging-update

Conversation

@galuszkak
Copy link
Copy Markdown
Contributor

Prior to starting a PR, please make sure you have read our
contributor tutorial.

Prior to a PR being reviewed, there needs to be a Github issue that the PR
addresses. Replace the brackets and text below with that issue number.

For #2015

Links to the line numbers/files in the OpenStack source code that support the
code in this PR:
https://docs.openstack.org/api-ref/message/?expanded=show-queue-details-detail

@galuszkak galuszkak force-pushed the feature/messaging-update branch from 71c48cb to f1c1974 Compare September 11, 2020 00:56
@coveralls
Copy link
Copy Markdown

coveralls commented Sep 11, 2020

Coverage Status

Coverage remained the same at 79.547% when pulling 2815a42 on galuszkak:feature/messaging-update into d0d95a0 on gophercloud:master.

@jtopjian
Copy link
Copy Markdown
Contributor

@galuszkak Thank you for submitting this.

Per the contributor tutorial linked above, API doc references aren't enough to determine if this is a valid OpenStack feature. Please see Step 3 specifically and let me know if you have any questions.

@galuszkak
Copy link
Copy Markdown
Contributor Author

Hi @jtopjian ,

If I understand your ask correctly does below proofs are enough, or there is something still needed?
https://github.com/openstack/zaqar/blob/master/zaqar/transport/validation.py#L344-L347
openstack/zaqar@e12c65a

Thanks for helping!

@jtopjian
Copy link
Copy Markdown
Contributor

@galuszkak Yes, that's perfect 🙂

One extra thing to consider here is that you have a combination of bool and omitempty. This means that a value of false will never be sent in the JSON request. According to the service-side code (https://github.com/openstack/zaqar/blob/master/zaqar/transport/validation.py#L344), the default value is false, so this combination should be fine. However, if you want the ability to explicitly send a value of false, this will need changed to *bool.

There's also some ListOpts fields you've added in this PR. We'll need to dig up the code references for those, too. Alternatively, you can leave them out of this PR and submit them in a new PR.

@galuszkak galuszkak force-pushed the feature/messaging-update branch from f1c1974 to 2815a42 Compare September 11, 2020 02:41
@galuszkak
Copy link
Copy Markdown
Contributor Author

@jtopjian thanks for feedback.

I've applied it. I will sent name and with_count in separate PR as it makes more sense to seperate those changes.

@theopenlab-ci
Copy link
Copy Markdown

theopenlab-ci bot commented Sep 11, 2020

Build succeeded.

Copy link
Copy Markdown
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.

LGTM - thank you!

@jtopjian jtopjian merged commit 98ddeee into gophercloud:master Sep 11, 2020
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.

3 participants