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.

Sign up for GitHub

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

Jump to bottom

ServiceBusAdministrationClient now uses _parse_conn_str from base_han… #14228

Merged
merged 4 commits into from
Oct 9, 2020
Merged

ServiceBusAdministrationClient now uses _parse_conn_str from base_han… #14228

merged 4 commits into from
Oct 9, 2020

Conversation

bradleydamato
Copy link
Contributor

…dlers

Hey @KieranBrantnerMagee @YijunXieMS I'm 90% sure that this passed all of the tests, here is a screenshot of both my invocation and the output post-test.

image

Also, let me know if I totally misunderstood you in terms of the changes to make!

@ghost ghost added Service Bus customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Oct 2, 2020
@ghost
Copy link

ghost commented Oct 2, 2020

Thank you for your contribution bradleydamato! We will review the pull request and get back to you soon.

@bradleydamato bradleydamato mentioned this pull request Oct 2, 2020
Closed
@bradleydamato
Copy link
Contributor Author

Hey @KieranBrantnerMagee did release get bumped to November or is that the next release?

@yunhaoling
Copy link
Contributor

hey @bradleydamato, thanks for the PR!
we just release 7.0.0b7 this week, next release would be in November.

Remove parse_conn_str() from common.utils and replace parse_conn_str(…
1e594dc 
…) with _parse_conn_str() from base_handler
@ghost
Copy link

ghost commented Oct 8, 2020 •
edited by ghost
Loading

CLA assistant check
All CLA requirements met.

@yunhaoling
Copy link
Contributor

/azp run python - servicebus - tests

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@yunhaoling
Copy link
Contributor

@bradleydamato, thanks for addressing my comments, looks good to me.
I've kicked off livetests against your PR, will approve once livetests get green.

@bradleydamato @yunhaoling
add underscore to base_handler import statement
307b86b 
new statement is:
`from ..._base_handler import _parse_conn_str`

Co-authored-by: Adam Ling (MSFT) <adam_ling@outlook.com>
@bradleydamato
Copy link
Contributor Author

bradleydamato commented Oct 8, 2020 •
edited
Loading

No pipelines are associated with this pull request.

A lot of these azure-pipeline checks ran when I first submitted the PR. Unsure if that is affecting them from running again. @yunhaoling for visibility.

@yunhaoling
Copy link
Contributor

@bradleydamato, don't worry, I can take this. (I'll try some commands, please ignore them :P)

@yunhaoling
Copy link
Contributor

/check-enforcer reset

@yunhaoling
Copy link
Contributor

/check-enforcer evaluate

@yunhaoling
Copy link
Contributor

/azp run python - servicebus - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yunhaoling
Copy link
Contributor

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yunhaoling
Copy link
Contributor

@bradleydamato, PR looks good to me, all tests passed.
Feel free to merge, Thanks again for your contribution!

@bradleydamato
Copy link
Contributor Author

Hey @yunhaoling thank you again for the thorough review! I would love to merge but it seems that I am not authorized to do so :/
55644FA8-C4C8-4C37-A9DF-5108F96CD722

@yunhaoling yunhaoling merged commit dad3cbe into Azure:master Oct 9, 2020
@yunhaoling
Copy link
Contributor

@bradleydamato, sorry I don't know you can't do it, I'll raise it to our team.

anyways, let me merge it for you. thanks!

@bradleydamato
Copy link
Contributor Author

Thank you @yunhaoling !!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yunhaoling yunhaoling yunhaoling approved these changes

@annatisch annatisch Awaiting requested review from annatisch

@KieranBrantnerMagee KieranBrantnerMagee Awaiting requested review from KieranBrantnerMagee

Assignees
No one assigned
Labels
customer-reported Issues that are reported by GitHub users external to the Azure organization. Service Bus
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bradleydamato @yunhaoling @KieranBrantnerMagee