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

add a new API version for component with a new properties #8434

Merged
merged 13 commits into from
Mar 31, 2020
Merged

add a new API version for component with a new properties #8434

merged 13 commits into from
Mar 31, 2020

Conversation

tofriedl
Copy link
Contributor

@tofriedl tofriedl commented Feb 19, 2020 •
edited
Loading

…urceId)

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.

@AutorestCI
Copy link

You don't have permission to trigger SDK Automation.
Please add yourself to Azure group from opensource portal if you are MSFT employee,
or please ask reviewer to add comment *** /openapibot sdkautomation ***.
Please ask [email protected] (or NullMDR in github) for additional help.

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 17, 2020 •
edited
Loading

azure-sdk-for-js - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 17, 2020 •
edited
Loading

azure-sdk-for-go - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 17, 2020 •
edited
Loading

azure-sdk-for-java - Release

️✔️ succeeded [Logs] [Expand Details]

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 17, 2020 •
edited
Loading

azure-cli-extensions - Release

No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 17, 2020 •
edited
Loading

azure-sdk-for-python - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from 537c072 with merge commit 4e000b4. SDK Automation 13.0.17.20200326.3
  • ️✔️azure-mgmt-applicationinsights [Logs]  [Release SDK Changes] 
    [build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
[build_package]   warnings.warn(msg)
[build_package] warning: no files found matching '*.py' under directory 'tests'
[build_package] warning: no files found matching '*.yaml' under directory 'tests'
[build_package] /usr/lib/python3.6/distutils/dist.py:261: UserWarning: Unknown distribution option: 'long_description_content_type'
[build_package]   warnings.warn(msg)
[build_package] warning: no files found matching '*.py' under directory 'tests'
[build_package] warning: no files found matching '*.yaml' under directory 'tests'
[breaking_change_setup] Ignoring mock: markers 'python_version <= "2.7"' don't match your environment
[breaking_change_setup] Cannot uninstall requirement azure-nspkg, not installed
[breaking_change_setup] Command '['/usr/local/bin/python', '-m', 'pip', 'uninstall', '-y', 'azure-nspkg']' returned non-zero exit status 1.
[ChangeLog] Size of delta 2.535% size of original (original: 50134 chars, delta: 1271 chars)
[ChangeLog] **Features**
[ChangeLog] 
[ChangeLog]   - Model ApplicationInsightsComponent has a new parameter retention_in_days
[ChangeLog]   - Model ApplicationInsightsComponent has a new parameter immediate_purge_data_on30_days
[ChangeLog]   - Model ApplicationInsightsComponent has a new parameter private_link_scoped_resources
[ChangeLog]   - Model ApplicationInsightsComponent has a new parameter connection_string
[ChangeLog]   - Model ApplicationInsightsComponent has a new parameter disable_ip_masking

@openapi-sdkautomation
Copy link

openapi-sdkautomation bot commented Mar 17, 2020 •
edited
Loading

azure-sdk-for-net - Release

️✔️ succeeded [Logs] [Expand Details]
  • ️✔️ Generate from 537c072 with merge commit 4e000b4. SDK Automation 13.0.17.20200326.3
  • ️✔️Microsoft.Azure.ApplicationInsights [Logs]  [Release SDK Changes]
      No Artifact Generated.

    This was referenced Mar 17, 2020
    Closed
    Closed
    Closed
    Closed
    @azure-pipelines
    Copy link

    Pull request contains merge conflicts.

    @azure-pipelines
    Copy link

    Pull request contains merge conflicts.

    @azure-pipelines
    Copy link

    Comment was made before the most recent commit for PR 8434 in repo Azure/azure-rest-api-specs

    @yoramsinger
    Copy link
    Contributor

    @zikalino can you review this one?

    @mmyyrroonn
    Copy link
    Contributor

    @zikalino Hello. Could you help check the python SDK readme as well? CLI wants multi-api version python sdk.

    @mmyyrroonn mmyyrroonn added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Mar 23, 2020
    @azure-pipelines
    Copy link

    Azure Pipelines successfully started running 1 pipeline(s).

    @tofriedl
    Copy link
    Contributor Author

    tofriedl commented Mar 23, 2020 via email

    @yoramsinger
    Copy link
    Contributor

    @pilor Can you review this please? Thanks.

    @mmyyrroonn
    Copy link
    Contributor

    @tofriedl Thanks. Please change examples as well and address ARM's comments.

    @azure-pipelines
    Copy link

    Azure Pipelines successfully started running 1 pipeline(s).

    @tofriedl
    Copy link
    Contributor Author

    @tofriedl Thanks. Please change examples as well and address ARM's comments.

    @tofriedl Thanks. Please change examples as well and address ARM's comments.

    @tofriedl Thanks. Please change examples as well and address ARM's comments.

    Reolved

    @azure-pipelines
    Copy link

    Azure Pipelines successfully started running 1 pipeline(s).

    @pilor pilor 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 WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Mar 25, 2020
    @arolshan arolshan changed the title add a new API version for component with a new property(WorkspaceReso… add a new API version for component with a new properties Mar 26, 2020
    @azure-pipelines
    Copy link

    Azure Pipelines successfully started running 1 pipeline(s).

    @azure-pipelines
    Copy link

    Azure Pipelines successfully started running 1 pipeline(s).

    @azure-pipelines
    Copy link

    Azure Pipelines successfully started running 1 pipeline(s).

    @tofriedl
    Copy link
    Contributor Author

    tofriedl commented Mar 29, 2020 via email

    @azure-pipelines
    Copy link

    Azure Pipelines successfully started running 1 pipeline(s).

    @mmyyrroonn
    Copy link
    Contributor

    @tofriedl So in the new api-version, service won't support it anymore. Can user get it? If user just cannot set it but they should be able to get it, I think we should mark it as read-only

    @tofriedl
    Copy link
    Contributor Author

    tofriedl commented Mar 30, 2020 via email

    @mmyyrroonn
    Copy link
    Contributor

    No. users can’t get it as well. Components in api-version 2020-02-02-preview will never have retentionInDays From: MyronFanQiu [email protected] Sent: Monday, March 30, 2020 11:48 To: Azure/azure-rest-api-specs [email protected] Cc: Tom Friedlander [email protected]; Mention [email protected] Subject: Re: [Azure/azure-rest-api-specs] add a new API version for component with a new properties (#8434) @tofriedlhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftofriedl&data=02%7C01%7CTom.Friedlender%40microsoft.com%7C8d8d2630fa46464c212508d7d4870588%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637211548690928120&sdata=%2BJaVpWrurxsZiG6db2PBjBWs3ogWm68rbhLN6%2BNCK0g%3D&reserved=0 So in the new api-version, service won't support it anymore. Can user get it? If user just cannot set it but they should be able to get it, I think we should mark it as read-only — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FAzure%2Fazure-rest-api-specs%2Fpull%2F8434%23issuecomment-605866716&data=02%7C01%7CTom.Friedlender%40microsoft.com%7C8d8d2630fa46464c212508d7d4870588%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637211548690928120&sdata=TGpXNNzTJ0q0OV9LznhJ%2FU7M6s97tDjRX6%2FD3SSQXkU%3D&reserved=0, or unsubscribehttps://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAMB2PDKJ3I7W5SFDUAUW7BTRKBMDHANCNFSM4KX2RKVQ&data=02%7C01%7CTom.Friedlender%40microsoft.com%7C8d8d2630fa46464c212508d7d4870588%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637211548690938116&sdata=Zkx74%2FUqYAmA4APwzpqimXLjf%2Bt9Ol%2BxDP9mfbhmfKs%3D&reserved=0.

    But I had a try and server did return this value to me.

    @tofriedl
    Copy link
    Contributor Author

    tofriedl commented Mar 31, 2020 via email

    @tofriedl
    Copy link
    Contributor Author

    @tofriedl So in the new api-version, service won't support it anymore. Can user get it? If user just cannot set it but they should be able to get it, I think we should mark it as read-only

    @tofriedl So in the new api-version, service won't support it anymore. Can user get it? If user just cannot set it but they should be able to get it, I think we should mark it as read-only

    @myronfanqiu
    If user already have retention it will keep on getting it.
    We are not going to delete data from components, but this number does not represent nothing because retention is calculated based on the Log Analytics workspace retention and not based on the Component retention.
    I think that if we will keep this value it can mislead users. (anyway they should know from ahead that the retention is calculated based on the workspace retention)

    One more point is – the default retention they have on the component schemas are the same as before. The change is that they cannot change it(they will get a BadRequest)

    @mmyyrroonn
    Copy link
    Contributor

    @tofriedl So in the new api-version, service won't support it anymore. Can user get it? If user just cannot set it but they should be able to get it, I think we should mark it as read-only

    @tofriedl So in the new api-version, service won't support it anymore. Can user get it? If user just cannot set it but they should be able to get it, I think we should mark it as read-only

    @myronfanqiu
    If user already have retention it will keep on getting it.
    We are not going to delete data from components, but this number does not represent nothing because retention is calculated based on the Log Analytics workspace retention and not based on the Component retention.
    I think that if we will keep this value it can mislead users. (anyway they should know from ahead that the retention is calculated based on the workspace retention)

    One more point is – the default retention they have on the component schemas are the same as before. The change is that they cannot change it(they will get a BadRequest)

    Based on your explanation, I think it's suitable to mark it as read-only. Anyway, it's fine to have it.

    @mmyyrroonn mmyyrroonn merged commit 4e000b4 into Azure:master Mar 31, 2020
    00Kai0 pushed a commit to 00Kai0/azure-rest-api-specs that referenced this pull request Oct 12, 2020
    @tofriedl @00Kai0
    add a new API version for component with a new properties (Azure#8434)
    75ead75
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @mmyyrroonn mmyyrroonn mmyyrroonn approved these changes

    @pilor pilor pilor approved these changes

    Assignees

    @zikalino zikalino

    Labels
    ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review
    Projects
    None yet
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    8 participants
    @tofriedl @AutorestCI @azuresdkci @yoramsinger @mmyyrroonn @pilor @zikalino @arolshan