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

[ServiceBus&EventHubs] api fixes #22418

Merged
merged 17 commits into from Jan 12, 2022
Merged

[ServiceBus&EventHubs] api fixes #22418

merged 17 commits into from Jan 12, 2022

Conversation

swathipil
Copy link
Member

updated the following, after API review:
EH:

  1. update EventData.body to type primitives (not Any): None/bool/int/float/bytes/str/dict/list/uuid.UUID

SB:
2) update type of kwarg session_id in sync/async ServiceBusClient.get_queue_receiver/get_subscription_receiver to Union[str, Literal[NEXT_AVAILABLE_SESSION]]
3) update return type of async/sync ServiceBusSession.get_state() to bytes
4) move docstring type of kwarg sub_queue to :paramtype line in async/sync ServiceBusClient.get_queue_receiver

@swathipil
Copy link
Member Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

annatisch
annatisch reviewed Jan 12, 2022
View reviewed changes
sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_client.py
@@ -5,6 +5,7 @@
from typing import Any, Union, Optional, TYPE_CHECKING
import logging
from weakref import WeakSet
from typing_extensions import Literal
Copy link
Member

Choose a reason for hiding this comment

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

As you mentioned in a comment elsewhere - I think this will need to be:

try:
    from typing import Literal
except ImportError:
    from typing_extensions import Literal

Copy link
Member Author

@swathipil swathipil Jan 12, 2022 •
edited

Choose a reason for hiding this comment

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

this causes a mypy error, same as this one: python/typeshed#3500

They mention checking python version as the solution. I'm not sure if that's what we want to do? or if we should leave this as is here since this works, though not ideal.

Copy link
Member

Choose a reason for hiding this comment

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

If we add the setup.py amendment above (which we probably should - we don't want unnecessary dependencies) then typing_extensions wont be available in <3.8.
As linked to within the issue you linked to - Mypy should support/understand Python version checks.

Copy link
Member Author

Choose a reason for hiding this comment

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

sounds good, thank you!

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like there's still an issue with mypy, but I think this is because this mypy check in the pipeline is using Python 3.7.

I can ask Scott about this.

sdk/servicebus/azure-servicebus/azure/servicebus/_servicebus_receiver.py
@@ -683,6 +685,8 @@ def receive_deferred_messages(
:caption: Receive deferred messages from ServiceBus.

"""
if kwargs:
warnings.warn(f"Unsupported keyword args: {kwargs}")
Copy link
Member

Choose a reason for hiding this comment

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

Is the f {} formatting support in 3.6+?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes it is

@swathipil
Copy link
Member Author

/azp run python - servicebus - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@swathipil swathipil merged commit d5e4a4c into Azure:main Jan 12, 2022
@swathipil swathipil deleted the swathipil/sb/fix-apiview-kwargs branch January 12, 2022 16:12
rakshith91 pushed a commit to rakshith91/azure-sdk-for-python that referenced this pull request Apr 10, 2022
@swathipil @rakshith91
[ServiceBus&EventHubs] api fixes (Azure#22418)
a464dd8 
* api updates

* fix mypy 3.7 errors

* Annas comments

* mypy literal variable error

* lint

* update type hint session_id

* docstring

* lint

* add SBSessionFilter to public API

* add typing_extensions to setup.py/add back kwargs with warnings

* add kwargs + warn to amqp message

* typing-extensions add to shared reqs

* lint

* add bool to body ED type

* anna + adam comments

* move to extras require

* add back to install reqs
azure-sdk pushed a commit to azure-sdk/azure-sdk-for-python that referenced this pull request Feb 14, 2023
@tadelesh @hoonality
@malincrist @changlong-liu @LFZ96
CodeGen from PR 22313 in Azure/azure-rest-api-specs
0884a68 
Release apimangement 2022-08 01 (Azure#22313)

* Release api management 2022 08 01 (Azure#21169)

* add base for 2022-08-01

* updates readme.md

* update api-version and examples

* resolve Go SDK breaking change

* update examples to fix model definition

Co-authored-by: Chenjie Shi <tadelesh.shi@live.cn>

* Adding Resolver Entities to documentation (Azure#21352)

* merging in resolver entities

* adding resolver updates

* fixed policy examples that were missing a policyId param

* fixed typo that added a nested properties prop

* fixed description for API Resolvers List, filter query

* fixed error in definitions for resolvers

* fixed example that had an incorrect response definition

* added missing json file references

* fix for linting errors

* ref fixes and undoing bad merge overwrites

* fix typo

* wiki for apis and products  (Azure#21595)

* wiki for apis and products

* prettier; fixed spellchecks

* Fixed spelling

* Updated path

Co-authored-by: changlong-liu <59815250+changlong-liu@users.noreply.github.com>

* code review changes; gihub checks fixes

* fixed conact name and paths

* added properties back

* added contract properties objects

* fixed lint

* changed example to match the definition

* prettier

* code review changes

* added paths and examples for wiki collections

* Added x-ms-pageable

* removed count

Co-authored-by: changlong-liu <59815250+changlong-liu@users.noreply.github.com>

* Add ConfirmConsentCode to APIM RP (Azure#22418)

* Update apimauthorizationproviders.json

Add ConfirmConsentCode endpoint

* Update definitions.json

* Create ApiManagementPostAuthorizationConfirmConsentCodeRequest.json

* Update definitions.json

Remove count property

* Update apimauthorizationproviders.json

* [2022-08-01] Fix Linter Errors (Azure#22537)

* linter errors

* prettier

* v2

* fix error response duplicate schema

* type object

* prettier

* sync data from current ga version

* remove duplicate parameter

* type object

---------

Co-authored-by: Chenjie Shi <tadelesh.shi@live.cn>
Co-authored-by: hoonality <92482069+hoonality@users.noreply.github.com>
Co-authored-by: malincrist <92857141+malincrist@users.noreply.github.com>
Co-authored-by: changlong-liu <59815250+changlong-liu@users.noreply.github.com>
Co-authored-by: Logan Zipkes <44794089+LFZ96@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yunhaoling yunhaoling yunhaoling left review comments

@annatisch annatisch annatisch approved these changes

@rakshith91 rakshith91 Awaiting requested review from rakshith91

Assignees
No one assigned
Labels
Event Hubs Service Bus
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@swathipil @annatisch @yunhaoling