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.

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

Introducing new version API 2019-05-01-preview with adding a new filter for usageDetails called publisherType #6575

Conversation

parisa-naeimi
Copy link
Contributor

Latest improvements:

MSFT employees can try out our new experience at OpenAPI Hub - one location for using our validation tools and finding your workflow.

Contribution checklist:

  • I have reviewed the documentation for the workflow.
  • Validation tools were run on swagger spec(s) and have all been fixed in this PR.
  • The OpenAPI Hub was used for checking validation status and next steps.

ARM API Review Checklist

  • Service team MUST add the "WaitForARMFeedback" label if the management plane API changes fall into one of the below categories.
  • adding/removing APIs.
  • adding/removing properties.
  • adding/removing API-version.
  • adding a new service in Azure.

Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.

  • If you are blocked on ARM review and want to get the PR merged urgently, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.
    Please follow the link to find more details on API review process.

parisa-naeimi and others added 4 commits July 9, 2019 13:33
…9-04-01-preview to version 2019-05-01-preview
2. Added filter section missing in UsageDetailsDownload and also added language parameter missing from specification/consumption/resource-manager/Microsoft.Consumption/preview/2019-05-01-preview/consumption.json
3. Updated Usage Detail Download example to have language parameter
@parisa-naeimi parisa-naeimi requested a review from kjeur as a code owner July 9, 2019 23:05
@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 9, 2019

SDK Automation [Logs] (Generated from 61f897a, Iteration 2)

Succeeded Go: Azure/azure-sdk-for-go [Logs] [Diff]
Succeeded Python: Azure/azure-sdk-for-python [Logs] [Diff]
Failed Java: Azure/azure-sdk-for-java [Logs] [Diff]
Failed JavaScript: Azure/azure-sdk-for-js [Logs] [Diff]
Failed Ruby: Azure/azure-sdk-for-ruby [Logs] [Diff]

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Jul 9, 2019

Automation for azure-sdk-for-python

The initial PR has been merged into your service PR:
Azure/azure-sdk-for-python#6395

@AutorestCI
Copy link

AutorestCI commented Jul 9, 2019

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

@AutorestCI
Copy link

AutorestCI commented Jul 9, 2019

Automation for azure-sdk-for-java

Encountered an unknown error: (azure-sdk-for-java)

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connection.py", line 159, in _new_conn
    (self._dns_host, self.port), self.timeout, **extra_kw)
  File "/usr/local/lib/python3.6/dist-packages/urllib3/util/connection.py", line 57, in create_connection
    for res in socket.getaddrinfo(host, port, family, socket.SOCK_STREAM):
  File "/usr/lib/python3.6/socket.py", line 745, in getaddrinfo
    for res in _socket.getaddrinfo(host, port, family, type, proto, flags):
socket.gaierror: [Errno -3] Temporary failure in name resolution

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connectionpool.py", line 600, in urlopen
    chunked=chunked)
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connectionpool.py", line 343, in _make_request
    self._validate_conn(conn)
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connectionpool.py", line 839, in _validate_conn
    conn.connect()
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connection.py", line 301, in connect
    conn = self._new_conn()
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connection.py", line 168, in _new_conn
    self, "Failed to establish a new connection: %s" % e)
urllib3.exceptions.NewConnectionError: <urllib3.connection.VerifiedHTTPSConnection object at 0x7fb6aceae668>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/requests/adapters.py", line 449, in send
    timeout=timeout
  File "/usr/local/lib/python3.6/dist-packages/urllib3/connectionpool.py", line 638, in urlopen
    _stacktrace=sys.exc_info()[2])
  File "/usr/local/lib/python3.6/dist-packages/urllib3/util/retry.py", line 399, in increment
    raise MaxRetryError(_pool, url, error or ResponseError(cause))
urllib3.exceptions.MaxRetryError: HTTPSConnectionPool(host='api.github.com', port=443): Max retries exceeded with url: /user (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7fb6aceae668>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution',))

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 33, in exception_to_github
    yield context
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 170, in rest_handle_action
    return rest_pull_close(body, restapi_repo, sdk_pr_target_repo, sdkbase, sdk_tag)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github.py", line 185, in rest_pull_close
    rest_pr_management(rest_pr, sdk_pr_target_repo, sdk_tag, sdk_default_base)
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/restapi/github_handler.py", line 151, in rest_pr_management
    sdk_tag=sdk_tag
  File "/usr/local/lib/python3.6/dist-packages/swaggertosdk/SwaggerToSdkNewCLI.py", line 254, in generate_sdk_from_git_object
    with manage_git_folder(gh_token, Path(temp_dir) / Path("rest"), branched_rest_api_id, pr_number=pr_number) as restapi_git_folder, \
  File "/usr/lib/python3.6/contextlib.py", line 81, in __enter__
    return next(self.gen)
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 272, in manage_git_folder
    clone_to_path(gh_token, temp_dir, split_git_id[0], branch_or_commit=branch, pr_number=pr_number)
  File "/usr/local/lib/python3.6/dist-packages/azure_devtools/ci_tools/github_tools.py", line 187, in clone_to_path
    login = user_from_token(gh_token).login
  File "/usr/local/lib/python3.6/dist-packages/github/AuthenticatedUser.py", line 230, in login
    self._completeIfNotSet(self._login)
  File "/usr/local/lib/python3.6/dist-packages/github/GithubObject.py", line 263, in _completeIfNotSet
    self._completeIfNeeded()
  File "/usr/local/lib/python3.6/dist-packages/github/GithubObject.py", line 267, in _completeIfNeeded
    self.__complete()
  File "/usr/local/lib/python3.6/dist-packages/github/GithubObject.py", line 272, in __complete
    self._url.value
  File "/usr/local/lib/python3.6/dist-packages/github/Requester.py", line 275, in requestJsonAndCheck
    return self.__check(*self.requestJson(verb, url, parameters, headers, input, self.__customConnection(url)))
  File "/usr/local/lib/python3.6/dist-packages/github/Requester.py", line 335, in requestJson
    return self.__requestEncode(cnx, verb, url, parameters, headers, input, encode)
  File "/usr/local/lib/python3.6/dist-packages/github/Requester.py", line 388, in __requestEncode
    status, responseHeaders, output = self.__requestRaw(cnx, verb, url, requestHeaders, encoded_input)
  File "/usr/local/lib/python3.6/dist-packages/github/Requester.py", line 412, in __requestRaw
    response = cnx.getresponse()
  File "/usr/local/lib/python3.6/dist-packages/github/Requester.py", line 114, in getresponse
    r = verb(url, headers=self.headers, data=self.input, timeout=self.timeout, verify=self.verify, allow_redirects=False)
  File "/usr/local/lib/python3.6/dist-packages/requests/sessions.py", line 546, in get
    return self.request('GET', url, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/requests/sessions.py", line 533, in request
    resp = self.send(prep, **send_kwargs)
  File "/usr/local/lib/python3.6/dist-packages/requests/sessions.py", line 646, in send
    r = adapter.send(request, **kwargs)
  File "/usr/local/lib/python3.6/dist-packages/requests/adapters.py", line 516, in send
    raise ConnectionError(e, request=request)
requests.exceptions.ConnectionError: HTTPSConnectionPool(host='api.github.com', port=443): Max retries exceeded with url: /user (Caused by NewConnectionError('<urllib3.connection.VerifiedHTTPSConnection object at 0x7fb6aceae668>: Failed to establish a new connection: [Errno -3] Temporary failure in name resolution',))

### Tag: package-preview-2019-04

These settings apply only when `--tag=package-preview-2019-04` is specified on the command line.

```yaml $(tag) == 'package-preview-2019-04'
``` yaml $(tag) == 'package-preview-2019-04'
Copy link
Member

Choose a reason for hiding this comment

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

Is there any particular reason to add space?

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 did not do any manual editing here. I just got the latest version and used openapi to generate a new version 2019-05-01-preview. But I can remove it anyways

@@ -1,7 +1,7 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

Please format this json file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, please let me know if it looks good now.

@erich-wang erich-wang added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Jul 10, 2019
@erich-wang
Copy link
Member

Please also address CI error:

parameter 'ln' is defined in global parameters section without 'x-ms-parameter-location' extension. This would add the parameter as the client property. Please ensure that this is exactly you want. If so, apply the extension "x-ms-parameter-location": "client". Else, apply the extension "x-ms-parameter-location": "method". at specification/consumption/resource-manager/Microsoft.Consumption/preview/2019-05-01-preview/consumption.json:3177

@parisa-naeimi
Copy link
Contributor Author

Please also address CI error:

parameter 'ln' is defined in global parameters section without 'x-ms-parameter-location' extension. This would add the parameter as the client property. Please ensure that this is exactly you want. If so, apply the extension "x-ms-parameter-location": "client". Else, apply the extension "x-ms-parameter-location": "method". at specification/consumption/resource-manager/Microsoft.Consumption/preview/2019-05-01-preview/consumption.json:3177

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Jul 10, 2019

SDK Automation [Logs] (Generated from 5de5a5e, Iteration 7)

Succeeded Go: Azure/azure-sdk-for-go [Logs] [Diff]
Succeeded Python: Azure/azure-sdk-for-python [Logs] [Diff]
Failed Java: Azure/azure-sdk-for-java [Logs] [Diff]
Succeeded JavaScript: Azure/azure-sdk-for-js [Logs] [Diff]
Failed Ruby: Azure/azure-sdk-for-ruby [Logs] [Diff]

@parisa-naeimi parisa-naeimi changed the title Dev consumption microsoft.consumption 2019 05 01 preview Introducing new version for API 2019-05-01-preview with adding a new filter for usageDetails called publisherType Jul 10, 2019
@parisa-naeimi parisa-naeimi changed the title Introducing new version for API 2019-05-01-preview with adding a new filter for usageDetails called publisherType Introducing new version API 2019-05-01-preview with adding a new filter for usageDetails called publisherType Jul 10, 2019
"$ref": "#/parameters/metricParameter"
},
{
"$ref": "#/parameters/languageParameter"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you introducing an explicit language parameter to this API? All ARM requests are supposed to respect the Accept-Language header as the requested language from the user.

https://github.com/Azure/azure-resource-manager-rpc/blob/70d10c715342149a3be94ebdc75ebde5ea0c9354/v1.0/common-api-details.md#client-request-headers

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In usageDetailsDownload the API caller needs to pass" ln=en" for instance as a url parameter. Then it is required. We are giving the URL to the CSV file we need to prepare the file based on the caller language specific. In previous PR it was not there, then I added it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand this is how your API was documented. To be compliant with ARM RPC, you should just use the Accept-Language header from the caller. Having an explicit 'ln' query parameter isn't needed. If you didn't make this change, whats the expected behavior when these 2 languages aren't the same?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ryansbenson: Thank you for the review and I removed language parameter from usage detail download and related examples based on your suggestion to be compatible with ARM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ryansbenson could you please review this again, I addressed your comments.

@ryansbenson ryansbenson added the ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review label Jul 11, 2019
@ms-premp
Copy link
Contributor

@ravbhatnagar can you please review this.
This is same as the older version we had. We are continuing with preview-version as discussed last time and discussed changes will be pulled in a new GA version. That work is still pending and hence this change is still in sync with the current preview version.

Copy link
Contributor

@KrisBash KrisBash 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

@KrisBash KrisBash removed ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Jul 17, 2019
@KrisBash KrisBash added the ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review label Jul 17, 2019
@parisa-naeimi
Copy link
Contributor Author

@erich-wang could you please let me know when can you check this, I already addressed your comments and ARM already signed off.

@parisa-naeimi
Copy link
Contributor Author

LGTM, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants