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

Fix network trace API REST format #4295

Merged
merged 12 commits into from
Oct 25, 2018
Merged

Fix network trace API REST format #4295

merged 12 commits into from
Oct 25, 2018

Conversation

michimune
Copy link
Contributor

This checklist is used to make sure that common issues in a pull request are addressed. This will expedite the process of getting your pull request merged and avoid extra work on your part to fix issues discovered during the review process.

PR information

  • The title of the PR is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For information on cleaning up the commits in your pull request, see this page.
  • Except for special cases involving multiple contributors, the PR is started from a fork of the main repository, not a branch.
  • If applicable, the PR references the bug/issue that it fixes.
  • Swagger files are correctly named (e.g. the api-version in the path should match the api-version in the spec).

Quality of Swagger

@michimune
Copy link
Contributor Author

@naveedaz Please review.

@AutorestCI
Copy link

AutorestCI commented Oct 19, 2018

Automation for azure-sdk-for-python

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

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Oct 19, 2018

Automation for azure-sdk-for-js

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

@AutorestCI
Copy link

AutorestCI commented Oct 19, 2018

Automation for azure-sdk-for-go

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

@AutorestCI
Copy link

AutorestCI commented Oct 19, 2018

Automation for azure-sdk-for-node

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

@praries880
Copy link

@azuresdkci add to whitelist

@praries880 praries880 added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Oct 23, 2018
@praries880
Copy link

@michimune I am requesting an ARM review of the API.

From what I could gather looking at the changes requested here, the API surface would remain the same (from the perspective of the generated SDK) but the behavior of what that generated function does is different. Its calling a new REST endpoint. This would break all existing customers and all of them would have to upgrade to a newer SDK for things to work as expected.

I an requesting the ARM folks to sign off on these changes.

@vladimirjoanovic
Copy link

This does seem like something that should be landed in a new API version.

@vladimirjoanovic vladimirjoanovic added ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Oct 23, 2018
@michimune
Copy link
Contributor Author

Actually the old API endpoints still exist. Only the swagger and SDKs want to point to a different REST endpoint. Do we still need a new API version in this case?

@michimune
Copy link
Contributor Author

Actually Navy told me what was wrong. I am working on fix.

@AutorestCI
Copy link

AutorestCI commented Oct 24, 2018

Automation for azure-sdk-for-ruby

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

@michimune
Copy link
Contributor Author

@vladimirjoanovic @praries880 I believe it's finally in a good shape. Automation says there aren't any breaking changes.

@vladimirjoanovic vladimirjoanovic added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed ARMChangesRequested <valid label in PR review process>add this label when require changes after ARM review labels Oct 25, 2018
@praries880
Copy link

@veronicagg can you kindly merge this PR?

@praries880 praries880 merged commit f47b6a2 into Azure:master Oct 25, 2018
@AutorestCI
Copy link

AutorestCI commented Oct 25, 2018

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/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 249, in generate_sdk_from_git_object
    with tempfile.TemporaryDirectory() as temp_dir:
  File "/usr/lib/python3.6/tempfile.py", line 931, in __init__
    self.name = mkdtemp(suffix, prefix, dir)
  File "/usr/lib/python3.6/tempfile.py", line 499, in mkdtemp
    prefix, suffix, dir, output_type = _sanitize_params(prefix, suffix, dir)
  File "/usr/lib/python3.6/tempfile.py", line 269, in _sanitize_params
    dir = gettempdir()
  File "/usr/lib/python3.6/tempfile.py", line 437, in gettempdir
    tempdir = _get_default_tempdir()
  File "/usr/lib/python3.6/tempfile.py", line 372, in _get_default_tempdir
    dirlist)
FileNotFoundError: [Errno 2] No usable temporary directory found in ['/tmp', '/var/tmp', '/usr/tmp', '/git-restapi']

@michimune
Copy link
Contributor Author

Thank you very much!

mccleanp pushed a commit that referenced this pull request Mar 23, 2022
* add new API version; add put/get AppRegInvitie API there

* update specification/azurelogistics/resource-manager/readme.md

* fix pipline error

* added provision state to appRegInvite

* fix ARM comments

Co-authored-by: Jeremiah Guo <[email protected]>
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.

5 participants