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

[text analytics] generate multiapi client, made v3.1-preview.1 default version #12720

Merged
merged 4 commits into from
Jul 29, 2020
Merged

[text analytics] generate multiapi client, made v3.1-preview.1 default version #12720

merged 4 commits into from
Jul 29, 2020

Conversation

iscai-msft
Copy link
Contributor

No description provided.

kristapratico
kristapratico reviewed Jul 24, 2020
View reviewed changes
...azure-ai-textanalytics/azure/ai/textanalytics/_generated/aio/_text_analytics_client_async.py
:keyword int polling_interval: Default waiting time between two polls for LRO operations if no Retry-After header is present.
"""

DEFAULT_API_VERSION = 'v3.0'
Copy link
Member

Choose a reason for hiding this comment

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

Based on the changelog blurb, should this be v3.1-preview.1?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

talked to @kristapratico about this offline. For the generated code I made default v3.0 to avoid syntaxerror's when calling the generated multiapi client, since v3.1-preview.1 added a parameter.

In the wrapped librnary, the default is v3.1-preview.1, meaning if a user doesn't pass in an api version, I pass v3.1-preview.1 to the generated multiapi client

kristapratico
kristapratico reviewed Jul 24, 2020
View reviewed changes
...azure-ai-textanalytics/azure/ai/textanalytics/_generated/aio/_text_analytics_client_async.py
"""

DEFAULT_API_VERSION = 'v3.0'
_PROFILE_TAG = "azure.ai.textanalytics.TextAnalyticsClient"
Copy link
Member

Choose a reason for hiding this comment

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

should this be "azure.ai.textanalytics.aio.TextAnalyticsClient"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, have opened an autorest bug on it: Azure/autorest.python#730

Shouldn't block this PR though

kristapratico
kristapratico reviewed Jul 24, 2020
View reviewed changes
sdk/textanalytics/azure-ai-textanalytics/azure/ai/textanalytics/_generated/v3_0/_metadata.json
@@ -0,0 +1,117 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

do we check this file in for multiapi?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The _metadata.json files are only used when creating the generated multiapi client, since they include the info that the multiapi client would need to know about each individual API. They have no use outside of that. We shouldn't remove them from the repo though, there's precedence (see mgmt plane network) for just keeping the files

kristapratico
kristapratico reviewed Jul 24, 2020
View reviewed changes
sdk/textanalytics/azure-ai-textanalytics/tests/test_multiapi_async.py
@@ -27,6 +27,6 @@ def test_v3_0_api_version(self, resource_group, location, text_analytics_account
@GlobalTextAnalyticsAccountPreparer()
def test_v3_1_preview_1_api_version(self, resource_group, location, text_analytics_account, text_analytics_account_key):
credential = AzureKeyCredential(text_analytics_account_key)
client = TextAnalyticsClient(text_analytics_account, credential, api_version=ApiVersion.V3_1_preview_1)
client = TextAnalyticsClient(text_analytics_account, credential, api_version=ApiVersion.V3_1_PREVIEW_1)
Copy link
Member

Choose a reason for hiding this comment

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

should we also make a call to verify? or do we test that somewhere else...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is an adequate test. The generated client's base url contains v3.0 or v3.1-preview.1 depending on the , and we use the base url as the base for all of our function calls

@iscai-msft
Copy link
Contributor Author

/check-enforcer evaluate

@iscai-msft iscai-msft requested a review from kristapratico July 29, 2020 18:42
@iscai-msft iscai-msft merged commit 7823466 into Azure:master Jul 29, 2020
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jul 29, 2020
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
40ae41c 
…into add_back_pii

* 'master' of https://github.com/Azure/azure-sdk-for-python: (40 commits)
  [text analytics] generate multiapi client, made v3.1-preview.1 default version (Azure#12720)
  Prevent AttributeError during get_certificate_version (Azure#12747)
  re-apply updates after resetting master (Azure#12771)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12745)
  Enable bandit for servicebus (Azure#12763)
  Skip pypy3 for service bus CI pipeline (Azure#12732)
  Update CODEOWNERS (Azure#12765)
  updated tox file (Azure#11087)
  Updating to match Microsoft recommended template (Azure#11045)
  [text analytics] update version in master (Azure#12749)
  Explicitly stringify message prints in samples per manual doc pass recommendation. (Azure#12709)
  Run only analyse dependency step for aggregate report (Azure#12728)
  VSCodeCredential supports Python 2.7 on Windows (Azure#12731)
  Enable bandit (Azure#12722)
  Move InteractiveCredential to a new module (Azure#12706)
  Add foundations for eng/common restructure (Azure#12725)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12730)
  fix flaky test (Azure#12721)
  python link verification (Azure#12543)
  [ServiceBus] Remove legacy control client (Azure#12644)
  ...
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jul 30, 2020
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
7dc065c 
…into opinion_mining

* 'master' of https://github.com/Azure/azure-sdk-for-python: (69 commits)
  [formrecognizer] renames from consistency check (Azure#12752)
  [text analytics] add back PII endpoint (Azure#12673)
  [text analytics] generate multiapi client, made v3.1-preview.1 default version (Azure#12720)
  Prevent AttributeError during get_certificate_version (Azure#12747)
  re-apply updates after resetting master (Azure#12771)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12745)
  Enable bandit for servicebus (Azure#12763)
  Skip pypy3 for service bus CI pipeline (Azure#12732)
  Update CODEOWNERS (Azure#12765)
  updated tox file (Azure#11087)
  Updating to match Microsoft recommended template (Azure#11045)
  [text analytics] update version in master (Azure#12749)
  Explicitly stringify message prints in samples per manual doc pass recommendation. (Azure#12709)
  Run only analyse dependency step for aggregate report (Azure#12728)
  VSCodeCredential supports Python 2.7 on Windows (Azure#12731)
  Enable bandit (Azure#12722)
  Move InteractiveCredential to a new module (Azure#12706)
  Add foundations for eng/common restructure (Azure#12725)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12730)
  fix flaky test (Azure#12721)
  ...
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jul 30, 2020
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
2bd6154 
…into regenerate_secrets

* 'master' of https://github.com/Azure/azure-sdk-for-python: (52 commits)
  [key vault] Regenerate keys (Azure#12101)
  [Tables] Azure Data Tables SDK, sync and async code (Azure#12766)
  fix doc, sample, docstring (Azure#12784)
  clean up links for PII samples (Azure#12826)
  [formrecognizer] renames from consistency check (Azure#12752)
  [text analytics] add back PII endpoint (Azure#12673)
  [text analytics] generate multiapi client, made v3.1-preview.1 default version (Azure#12720)
  Prevent AttributeError during get_certificate_version (Azure#12747)
  re-apply updates after resetting master (Azure#12771)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12745)
  Enable bandit for servicebus (Azure#12763)
  Skip pypy3 for service bus CI pipeline (Azure#12732)
  Update CODEOWNERS (Azure#12765)
  updated tox file (Azure#11087)
  Updating to match Microsoft recommended template (Azure#11045)
  [text analytics] update version in master (Azure#12749)
  Explicitly stringify message prints in samples per manual doc pass recommendation. (Azure#12709)
  Run only analyse dependency step for aggregate report (Azure#12728)
  VSCodeCredential supports Python 2.7 on Windows (Azure#12731)
  Enable bandit (Azure#12722)
  ...
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jul 30, 2020
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
20d94bd 
…into regenerate_certs

* 'master' of https://github.com/Azure/azure-sdk-for-python:
  [key vault] Regenerate keys (Azure#12101)
  [Tables] Azure Data Tables SDK, sync and async code (Azure#12766)
  fix doc, sample, docstring (Azure#12784)
  clean up links for PII samples (Azure#12826)
  [formrecognizer] renames from consistency check (Azure#12752)
  [text analytics] add back PII endpoint (Azure#12673)
  [text analytics] generate multiapi client, made v3.1-preview.1 default version (Azure#12720)
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jul 31, 2020
@iscai-msft
Merge branch 'master' of https://github.com/Azure/azure-sdk-for-python …
6cd0f4f 
…into ta_opinion_mining_sample

* 'master' of https://github.com/Azure/azure-sdk-for-python: (47 commits)
  [text analytics] opinion mining support (Azure#12542)
  [key vault] Regenerate keys (Azure#12101)
  [Tables] Azure Data Tables SDK, sync and async code (Azure#12766)
  fix doc, sample, docstring (Azure#12784)
  clean up links for PII samples (Azure#12826)
  [formrecognizer] renames from consistency check (Azure#12752)
  [text analytics] add back PII endpoint (Azure#12673)
  [text analytics] generate multiapi client, made v3.1-preview.1 default version (Azure#12720)
  Prevent AttributeError during get_certificate_version (Azure#12747)
  re-apply updates after resetting master (Azure#12771)
  Sync eng/common directory with azure-sdk-tools repository (Azure#12745)
  Enable bandit for servicebus (Azure#12763)
  Skip pypy3 for service bus CI pipeline (Azure#12732)
  Update CODEOWNERS (Azure#12765)
  updated tox file (Azure#11087)
  Updating to match Microsoft recommended template (Azure#11045)
  [text analytics] update version in master (Azure#12749)
  Explicitly stringify message prints in samples per manual doc pass recommendation. (Azure#12709)
  Run only analyse dependency step for aggregate report (Azure#12728)
  VSCodeCredential supports Python 2.7 on Windows (Azure#12731)
  ...
openapi-sdkautomation bot pushed a commit to AzureSDKAutomation/azure-sdk-for-python that referenced this pull request Feb 1, 2021
CodeGen from PR 12720 in Azure/azure-rest-api-specs
593f4b4 
cli onboard mixedreality (Azure#12720)

* CLI onboard mixedreality

* brief params

* revert since fixed in modelerfour
@iscai-msft iscai-msft deleted the multiapi_client_with_v3_1_preview_1_default branch September 10, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kristapratico kristapratico kristapratico approved these changes

@lmazuel lmazuel Awaiting requested review from lmazuel

@rakshith91 rakshith91 Awaiting requested review from rakshith91

@johanste johanste Awaiting requested review from johanste

Assignees
No one assigned
Labels
Cognitive - Text Analytics
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@iscai-msft @kristapratico