-
Notifications
You must be signed in to change notification settings - Fork 61
fix default polling method listed in docstring #830
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.
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
Conversation
| :keyword polling: True for ARMPolling, False for no polling, or a | ||
| polling object for personal polling strategy | ||
| :paramtype polling: bool or ~azure.core.polling.{{ "Async" if async_mode else "" }}PollingMethod | ||
| {% set default_polling_method = "ARMPolling"if code_model.options['azure_arm'] else "LROBasePolling" %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Main point of this PR (sorry there's a lot of fluff where I add a keyword to dedup code in the LRO world). Before, we would always generate ARMPolling as the default polling method, even when arm flag wasn't set
lmazuel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make this with a default, directive and namer
…into improve_lro_docstrings * 'autorestv3' of https://github.com/Azure/autorest.python: add typing imports to multiapi client and config (#794) deduplicate model serialization code (#828) remove azure-arm flag when generating paging (#829) clean up tasks file (#827)
| ), | ||
| } | ||
|
|
||
| operation["extensions"]["default-no-polling-method-sync"] = "azure.core.polling.NoPolling" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added the default no polling, and the base polling method. to the namer as well. I'm not exposing a directive to override these (at least not yet, there seems to be no use case for it). But I wanted all of the LRO paths to be in one area, and it cleaned up the code in the LROOperation model class
|
|
||
| def get_poller_path(self, async_mode: bool) -> str: | ||
| extension_name = "poller-async" if async_mode else "poller-sync" | ||
| extension_name = "poller-{}sync".format("a" if async_mode else "") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it really improve the readability? :p
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made some fixes for readability
lmazuel
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds fair
…into improve_lro_docstrings * 'autorestv3' of https://github.com/Azure/autorest.python: add enum description son top of declaration, wrap comments (#844) add default value 'api-key' for `--credential-key-header-name` (#840) update head-as-boolean spec
…into black_plugin * 'autorestv3' of https://github.com/Azure/autorest.python: fix default polling method listed in docstring (#830) add enum description son top of declaration, wrap comments (#844) add default value 'api-key' for `--credential-key-header-name` (#840) update head-as-boolean spec
…into docs * 'autorestv3' of https://github.com/Azure/autorest.python: add black flag (#836) fix default polling method listed in docstring (#830) add enum description son top of declaration, wrap comments (#844)
…into paging_separate_operations * 'autorestv3' of https://github.com/Azure/autorest.python: add black flag (#836) fix default polling method listed in docstring (#830)
fixes #782