Skip to content

Enable preparers to implicitly handle async tests#10394

Merged
KieranBrantnerMagee merged 16 commits intoAzure:masterfrom
KieranBrantnerMagee:kibrantn/servicebus/preparer-implicit-async-handling
Mar 23, 2020
Merged

Enable preparers to implicitly handle async tests#10394
KieranBrantnerMagee merged 16 commits intoAzure:masterfrom
KieranBrantnerMagee:kibrantn/servicebus/preparer-implicit-async-handling

Conversation

@KieranBrantnerMagee
Copy link
Member

Currently, devs have to wrap async tests in an event loop to be called synchronously if they wish to utilize preparers; requiring them to get argument trimming and such right as well.
This PR enables this functionality within the core preparer, attempting to import asyncio and validate the function signature, wrapping if necessary, before falling back to a typical call pattern.

No work is necessary for sdks using a wrapper or similar approach, they can be deprecated as time allows/new tests can simply be written in async without concern or additional boilerplate.

Note: @praveenkuttappan I split out the unit tests for this preparer functionality in an async_tests subfolder, as with our normal SDKs, so that it will not interfere with other tests (e.g. syntax error pre 3.5). I'm paranoid however since this is both the first of such tests in this space, and not in one of the typical SDK paths, so I just wanted to sanity check the steps I took past someone beforehand. If we have a pipe I can run to validate this don't hesitate to let me know so I can take any effort off you.

…ent resource is cached if a child is cached, otherwise an exception will be thrown before test run)

- Aggregate cache key with parents, such that, for instance, a service bus underneath a EU RG won't be swapped for a service bus underneath an NA RG. (Previously only took into account child cache settings to look up the child in the cache)
- Switch cache usage in test classes to use decorators for ease, add helper to resource group preparer.
- Clean up logging and documentation, tweaks to ensure pylint is happy.
- Add azure-devtools to dev-requirements
- Remove spurious delete cleanup code.
remove the need for a distinct cache_pending_deletes structure
update servicebus dev requirements
add random_name_enabled to resource_testcase cached helper wrapper.
fix bug in how trimmed_kwargs are used such that we do not trim the kwargs that we pass to deletion, since we may remove elements needed for removal.
… into creation failure exceptions to compensate, as the primary goal of the additional logging was to ensure users saw all possible details of what is typically a Critical Failure.
…outine, falling back to synchronous if not.

This allows use of preparers without an additional decorator to wrap the function, and obsoletes pytest.mark.asyncio for preparers.
Add an additional exception check to the async fallback, for certain deployments of 2.7 where this is the response when trying to import asyncio that has been pip installed.
@KieranBrantnerMagee KieranBrantnerMagee added the Client This issue points to a problem in the data-plane of the library. label Mar 20, 2020
@KieranBrantnerMagee KieranBrantnerMagee self-assigned this Mar 20, 2020
@adxsdk6
Copy link

adxsdk6 commented Mar 20, 2020

Can one of the admins verify this patch?

lmazuel
lmazuel previously approved these changes Mar 20, 2020
@KieranBrantnerMagee KieranBrantnerMagee merged commit 61ef6fa into Azure:master Mar 23, 2020
KieranBrantnerMagee added a commit to KieranBrantnerMagee/azure-sdk-for-python that referenced this pull request Mar 26, 2020
* Add a clause in the preparer to attempt to consume fn as an async coroutine, falling back to synchronous if not.  This allows use of preparers without an additional decorator to wrap the function.
* Add unit tests for the preparer async handling capabilities
* Add an additional exception check to the async fallback, for certain deployments of 2.7 where this is the response when trying to import asyncio that has been pip installed.
* Port storage async tests to give an example of the new async preparer working on another test suite.
yunhaoling added a commit that referenced this pull request Apr 3, 2020
* draft track2 sync client implementation

* add retry and backoff for send and receive

* improve retry and implement iterator for receiver

* improve sync implementation

* draft ClientBaseAsync and SenderClientAsync implementation

* improve structure and implement async receiver client

* add receiver client file

* sync version constructor improvement

* add sync samples

* async implementation

* add async sample

* refactor mixin and implement receiving deferred message

* add receiving defer message sample

* adjust method position

* optimize async and implement receive deferred message

* adjust backoff method name

* Update readme for track 2 preview 1 release

* add sync client docstring

* Convert to the modernized README structure.

* async client docstring

* Remove en-us localized links
take inspiration from EventHubs readme for primary links, and logging and feedback subsections.
Adjust version ID
Add changelog stanza stub
Adjust samples readme
Remove top level create_queue and troubleshooting-for-that-endpoint mentions for now until that's sorted.

* Remove 3.4 from acceptable python version list
Improve readme env var naming clarity, as well as cloud shell CLI verbiage.

* introduce new top level servicebus client, rename old client design, update samples

* split message types

* Add python 3.8 to acceptable versions

* Fix broken changelog URL markdown

was brackets, needed to be parens.

* update retry part

* add mgmt retry and update get_xxx factory methods

* add renew_lock back to deferred message

* remove track1 client/sender/receiver

* fix bug in sample code and logging

* remove t1 sample code

* fix pylint

* update __init__.py to relative reference

* add new docstring sample

* update docstring for the code

* move session id to message

* bring back the hotfix for ReceiveAndDetele

* send batch feature

* add peek back and fix pylint

* connection sharing in top level client

* open handler in the mgmt request

* merge deferred message into received message

* draft schedule and cancel implementation

* Initial test conversion work, port sync tests, note failures with TODOS, fix bugs elsewhere. (settlement mode plumbing, idle_timeout plumbing, session_id plumbing, add reconnect to handler, expose AutoReconnect)

* update common to _common and update module import accordingly

* update according to reviews

* update code according to comments

* session receive support

* Remove spurious comments into a formal doc, clean up tests and imports, preliminary session test porting.

* add ServiceBusSession support

* Adjust with exception statements to the new and improved throw-at-client-init.  108
Remove spurious TODOS from sb_client into a proper doc.

* reenable batching

* Make doc review comments to readme; adding links for key concepts, readding troubleshooting and putting logging under it, fixing URIs, adjusting header levels, and making samples fully copy-pastable

* improve connection sharing for entity-specific conn str, improve session related function

* add retry for iterator

* Make PR comments.
Change deadletter sample to defer
remove some EH bits left over.
Have migration guide include batch logic.
Add documentation to point to portal namespace creation.

* specify error type when entity mismatch and specify auth error in create_exception

* Adjust code post-merge to accomodate new path names and package structure

* Fix variable naming bugs,
put missing properties on servicebussession and access them properly,
adjust session tests accordingly
left notes of failures (these will have to be removed for final PR)

* fix bug when resetting the error

* idle timeout fix

* fix message settled not being set properly

* fix bug in session and improve error handling

* add session_id into message constructor

* revert to separate connection

* fix several bugs in session implementation and error handling

* add session expiration check

* Adjust tests after latest T2 changes, including making a public accessor for locked_until, catching some exceptions if a session isn't present on a receiver, and tweaking exception types and property paths.

* add comment for session locked until property

* preparer resource caching framework (#10126)

* Initial commit of preparer resource caching functionality.  Demonstrates use on the servicebus queue tests.  This will allow tests that opt-in to utilize resources cached from previous equivalent resource creation steps, to improve test runtime.  Deletion is delayed until the end of all pending tests, via a session fixture.
* Inferred caching is currently disabled. (You have to explicitly declare a parent resource is cached if a child is cached, otherwise an exception will be thrown before test run)
* Aggregates cache key with parents, such that, for instance, a service bus underneath a EU RG won't be swapped for a service bus underneath an NA RG.
* Switch cache usage in test classes to use decorators for ease, add helper to resource group preparer.
* Clean up preparer logging and documentation, tweaks to ensure pylint is happy.
* Add azure-devtools to servicebus dev-requirements (to allow local test runs to properly consume preparer logic)
* add random_name_enabled to resource_testcase cached helper wrapper so that this functionality is leveraged by default (solves a lot of engsys problems with resource collision)
* fix bug in how trimmed_kwargs are used such that we do not trim the kwargs that we pass to deletion, since we may remove elements needed for removal.
* Move resource cache cleanup into base conftest
* Default preparer logging to warn and above to ensure visibility of potential failures.
* Preparer logging is not on by default, but pushed more detail into creation failure exceptions to compensate, as the primary goal of the additional logging was to ensure users saw all possible details of what is typically a Critical Failure.
* Add longer timeout to tests.yml to support still-lengthy sb tests, make preparer exception catching more comprehensive
* Add unit tests for the caching preparer.  Reverse the deferred delete order to try and emulate as closely as possible the first-in-last-out deletion behavior from non-cached resources.

* Enable preparers to implicitly handle async tests (#10394)

* Add a clause in the preparer to attempt to consume fn as an async coroutine, falling back to synchronous if not.  This allows use of preparers without an additional decorator to wrap the function.
* Add unit tests for the preparer async handling capabilities
* Add an additional exception check to the async fallback, for certain deployments of 2.7 where this is the response when trying to import asyncio that has been pip installed.
* Port storage async tests to give an example of the new async preparer working on another test suite.

* add prefech back

* fix check session expiry error and align the behavior of prefetch with T1

* fix bug in async receiver close

* begin converting async queue tests to T2
make small fixes to repair flaky test_queue tests

* Add reconnect to async base handler
Begin porting async tests
small tweaks to sync tests to adjust according to the base sdk changes

* remove test skip on now-passing test.

* move _open from get_receiver/sender into contextmgr

* Tweak test notes

* enable test to be cached in async queue test

* normalize caching preparers, and change session->session_id

* update samples and code snippets

* Fix async tests (batch addition, running/open naming) and enable logging to try and catch memory access failure.

* fix bug when a handler dont get closed if it failed to open

* continue removing todos as more tests become functional.

* orphan message error type changed from ValueError to MessageSettleFailed

* Update exception catches with latest changes.  Use open_with_retry rather than just open to get proper "close-on-failure" behavior

* Move utility methods into separate file.
Write new stress testing infra that ties into pytest/preparers.
convert existing stress tests (reconnecttest still pending)

* PR comments
change debug -> logging_enable
Remove reconnect, is now implicit.

* Fix small lingering bugs in stress_test,
make duration overrideable from command line,
remove historical stress tests now that they're ported.

* improve stress test result printout

* fix ci bug

* fix syntax error

* bump version to v7 and fix syntax error in py3.5

* Adjust version to 7
Bolster migration guide
Flesh out changelog with features vs breaking changes, long-form.

* Add additional detail to pending-features list in changelog

* Adjust sample readme product name

* add utc suffix to time relevant variables and parameters, fix pylint and mypy, move unused code

* change MessageSettleFailed to MessageLockExpired where appropriate

* add reson for dead_letter according to service spec

* constify strings

* more string constifying

* add ivar docstring and remove async message lock token

* fix bug in kwargs and improve from_connection_string implementation

* add docstring, update async_message to be private module

* convert tests to timezone aware datetimes
ensure Azure capitalization in docs
Correct changelog verbiagei

* make control_client module private

* remove aio module duplicates

* rename timeout to max_wait_time in receive method

* Make test timezone usage be py27 compatible via consuming the util utc_now helper.
Fix constant MGMT_RESPONSE_EXPIRATION expiration->expirations

* update control_client to _contral_client

* rename message_timeout to timeout in send to align with EH

* make queue_name and topic_name docstrings more explicit about collision case.

* Message expiration and Receiver expiration are subtlely different constants, the former being pluralized.

* Readd buildtargetingstring that was removed in a merge
Remove duplicate setcache call in sb preparer

* set logging_enable=False by default for tests to not mess up OSX

* update migration_guide

* remove additional dev_requirement from azure-mgmt-servicebus

Co-authored-by: Kieran Brantner-Magee <kibrantn@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Client This issue points to a problem in the data-plane of the library.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants