Skip to content

Conversation

@v-Ajnava
Copy link

@v-Ajnava v-Ajnava commented Apr 6, 2018

Test Fixes

@v-Ajnava v-Ajnava requested a review from lmazuel as a code owner April 6, 2018 16:39
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

1 similar comment
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@lmazuel lmazuel force-pushed the restapi_auto_servicebus/resource-manager branch from a8e1ca1 to 8ed2362 Compare April 20, 2018 22:23
Copy link
Member

@lmazuel lmazuel left a comment

Choose a reason for hiding this comment

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

General advice in Python, if you touch a _variable name it's probably the wrong thing to do.

Copy link
Member

Choose a reason for hiding this comment

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

Don't do that, that's really bad. And it's why the CI is breaking.
If you wish not to wait, use polling=False. If you wish to wait, call result()

Copy link
Author

Choose a reason for hiding this comment

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

Resolved

Copy link
Member

Choose a reason for hiding this comment

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

Don't do that, that's really bad. And it's why the CI is breaking.
If you wish not to wait, use polling=False. If you wish to wait, call result()

Copy link
Author

Choose a reason for hiding this comment

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

Resolved

Copy link
Member

Choose a reason for hiding this comment

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

If namespace is a long running operation, calling result() will wait and you don't need that. There is automatic blocking on result()

Copy link
Author

Choose a reason for hiding this comment

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

Resolved

Copy link
Member

Choose a reason for hiding this comment

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

Don't do that, that's really bad. And it's why the CI is breaking.
If you wish not to wait, use polling=False. If you wish to wait, call result()

Copy link
Author

Choose a reason for hiding this comment

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

Resolved

Copy link
Member

Choose a reason for hiding this comment

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

Don't do that, that's really bad. And it's why the CI is breaking.
If you wish not to wait, use polling=False. If you wish to wait, call result()

Copy link
Author

Choose a reason for hiding this comment

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

Resolved

Copy link
Member

Choose a reason for hiding this comment

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

Don't do that, that's really bad. And it's why the CI is breaking.
If you wish not to wait, use polling=False. If you wish to wait, call result()

Copy link
Author

Choose a reason for hiding this comment

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

Resolved

@lmazuel
Copy link
Member

lmazuel commented Apr 20, 2018

@v-Ajnava There is strong issues in your tests, that then have no chances to pass CI.
Note that I tried to fix your PR by playing with commits and do push --force on your branch. You must reset --hard it. If you don't know how to do that, just delete your local SDK folder and re-clone from Github your fork.

@codecov-io
Copy link

codecov-io commented Apr 21, 2018

Codecov Report

❗ No coverage uploaded for pull request base (restapi_auto_servicebus/resource-manager@8ed2362). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@                             Coverage Diff                             @@
##             restapi_auto_servicebus/resource-manager    #2357   +/-   ##
===========================================================================
  Coverage                                            ?   54.86%           
===========================================================================
  Files                                               ?     5552           
  Lines                                               ?   129160           
  Branches                                            ?        0           
===========================================================================
  Hits                                                ?    70858           
  Misses                                              ?    58302           
  Partials                                            ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ed2362...65fca82. Read the comment docs.

@v-Ajnava
Copy link
Author

@lmazuel , My Bad, did not figured that .result() will get namespace resource. rather I went in wrong direction to resolve it. sorry for the trouble.
I have reset --hard to 'restapi_auto_servicebus/resource-manager', made changes and pushed.

Copy link
Member

@lmazuel lmazuel left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@lmazuel lmazuel merged commit f0471b4 into Azure:restapi_auto_servicebus/resource-manager Apr 24, 2018
AutorestCI pushed a commit that referenced this pull request Apr 26, 2018
* Generated from 53d1a0f7b7527828b2cdec46ed4fe3af768b11f4 (#1946)

updated POST opretions for Authorization rules for Queues and Topics

* Generated from eb292024b547ba2920c6cbe2231e8ee44a2c71ef (#2049)

Added enableBatchedOperations property to ServiceBus Queue

* Fix double azure-common install that breaks bdist_wheel

* Generated from bc96045bf6a0e056903d0a2d798202ff9bd05165 (#2249)

added 'properties' to corelationfilter

* Generated from cb47714a72869d9271ef4a750f5997d4d070aaf2 (#2282)

removed 'enableSubscriptionPartitioning' depricated property

* Update version.py

* Updated Tests
lmazuel added a commit that referenced this pull request Apr 27, 2018
* Generated from eb292024b547ba2920c6cbe2231e8ee44a2c71ef (#2049)

Added enableBatchedOperations property to ServiceBus Queue

* Generated from bc96045bf6a0e056903d0a2d798202ff9bd05165 (#2249)

added 'properties' to corelationfilter

* Generated from cb47714a72869d9271ef4a750f5997d4d070aaf2 (#2282)

removed 'enableSubscriptionPartitioning' depricated property

* Update version.py

* test fixes (#2357)

* Generated from 53d1a0f7b7527828b2cdec46ed4fe3af768b11f4 (#1946)

updated POST opretions for Authorization rules for Queues and Topics

* Generated from eb292024b547ba2920c6cbe2231e8ee44a2c71ef (#2049)

Added enableBatchedOperations property to ServiceBus Queue

* Fix double azure-common install that breaks bdist_wheel

* Generated from bc96045bf6a0e056903d0a2d798202ff9bd05165 (#2249)

added 'properties' to corelationfilter

* Generated from cb47714a72869d9271ef4a750f5997d4d070aaf2 (#2282)

removed 'enableSubscriptionPartitioning' depricated property

* Update version.py

* Updated Tests

* [AutoPR servicebus/resource-manager] Service Bus: added $skip and $top to list commands (#2472)

* Generated from 8dcd1fb891075f38d26f230c0d3078629ba6b34f

added $skip and $top to list commands

* Generated from 69e26f29dd3395f76d8b5cc8852f25506e9d7697

removed skip and top for namespaces

* Generated from 0d81d201fe6d5174be33f52d93c73d827c53df46 (#2477)

Py conf SB 0.5.0

* SB Mgmt 0.5.0 packaging
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.

5 participants