[App Insights] Sync to swagger, bump to 1.0.0-preview-1, rename package breaking changes #4790

alexeldeib · 2018-09-24T21:42:07Z

Description

Azure/azure-rest-api-specs#3771
Azure/azure-rest-api-specs#3084 --> big one
Azure/azure-rest-api-specs#3385
Azure/azure-rest-api-specs#3997

This checklist is used to make sure that common guidelines for a pull request are followed.

Please add REST spec PR link to the SDK PR
I have read the contribution guidelines.
The pull request does not introduce breaking changes.

General Guidelines

Title of the pull request 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 more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

Pull request includes test coverage for the included changes.

SDK Generation Guidelines

If an SDK is being regenerated based on a new swagger spec, a link to the pull request containing these swagger spec changes has been included above.
The generate.cmd file for the SDK has been updated with the version of AutoRest, as well as the commitid of your swagger spec or link to the swagger spec, used to generate the code.
The *.csproj and AssemblyInfo.cs files have been updated with the new version of the SDK.

…Preview-1

alexeldeib · 2018-09-24T22:08:23Z

I've been refining the data plane swagger for Application Insights and working towards 1.0.0 release for all of our SDKs. This PR rolls up a few changes I've made:

break Metrics, Events, and Queries into separate operation groups
minor operation renaming
move appId parameter from client to method (quality of life; makes sense to new up one client and query with different parameters)
various type fixes: some timespans need to be strings, some enums should be refined/loosened.

Adding on to that, this is an SDK to query data plane information -- metrics, logs and events for Azure resources. Application Insights also has numerous clients for sending telemetry to Azure, e.g. Microsoft.ApplicationInsights.
After discussing this with App Insights folks and Azure SDK PMs, I'd like to propose renaming this package Microsoft.Azure.ApplicationInsights.Query, to clarify any ambiguity between the two. I've already discussed and begun implementing similar changes with Python, Java, and Node SDKs.

I've made the relevant code changes already, but I wanted to ask how to go about this procedurally? Is there a precedent for package rename? Could we e.g., leave a note on the Microsoft.Azure.ApplicationInsights package page with a deprecation notice pointing to the newly named package? Would love to get thoughts on this.

Thanks!

alexeldeib · 2018-09-24T22:43:11Z

Incomplete

Update comments for documentation to reflect AppID parameter
Changelog

dsgouda · 2018-09-24T22:31:48Z

...ationInsights/DataPlane/ApplicationInsights/Microsoft.Azure.ApplicationInsights.Query.csproj

-    <AssemblyName>Microsoft.Azure.ApplicationInsights</AssemblyName>
-    <PackageId>Microsoft.Azure.ApplicationInsights</PackageId>
-    <PackageTags>Management.ApplicationInsights;</PackageTags>
+    <VersionPrefix>1.0.0.0</VersionPrefix>


versioning should be 1.0.0
Please rename VersionPrefix with Version

dsgouda · 2018-09-24T22:32:56Z

...ationInsights/DataPlane/ApplicationInsights/Microsoft.Azure.ApplicationInsights.Query.csproj

    <PackageReleaseNotes>This is a preview release of the Application Insights Data SDK.</PackageReleaseNotes>
  </PropertyGroup>
  <PropertyGroup>
    <TargetFrameworks>net452;netstandard1.4</TargetFrameworks>
-    <RootNamespace>Microsoft.Azure.ApplicationInsights</RootNamespace>
+    <RootNamespace>Microsoft.Azure.ApplicationInsights.Query</RootNamespace>
+    <Version>1.0.0-Preview-1</Version>


Please remove this

dsgouda · 2018-09-24T22:33:29Z

...ationInsights/DataPlane/ApplicationInsights/Microsoft.Azure.ApplicationInsights.Query.csproj

-    <PackageId>Microsoft.Azure.ApplicationInsights</PackageId>
-    <PackageTags>Management.ApplicationInsights;</PackageTags>
+    <VersionPrefix>1.0.0.0</VersionPrefix>
+    <VersionSuffix>Preview-1</VersionSuffix>


Please remove this
Also, is the package stable or preview?
If this is still preview, the version can be bumped to 0.10.0-preview instead

dsgouda · 2018-09-24T22:34:37Z

src/SDKs/_metadata/ApplicationInsights_data-plane.txt

-GitHub user: Azure
-Branch:      master
-Commit:      d122af3086f0b1693b9c506befc497a1f4a8b8e1
+GitHub user: alexeldeib


Is the spec merged?

There's an open PR which changes an enum to modelAsString. Rest of the changes are merged.

I can regenerate after it's merged. The same PR contains the namespace change.

dsgouda · 2018-09-24T22:52:40Z

@alexeldeib As such you cannot rename a package, we can hide/deprecate the old package with a message that redirects to a new package and then publish whichever new packages are needed. Please start an email thread if you need further details

alexeldeib · 2018-09-25T00:55:13Z

@dsgouda confirming, for preview version you want 0.10.0-preview not 1.0.0-preview? I made it 0.10.0-preview currently.

dsgouda · 2018-09-25T15:24:14Z

@alexeldeib there should be a strong reason for major version bumps (if the REST spec version is new or if breaking changes are introduced to a stable version)
This looks like a preview version without revisions to the REST spec version itself, so 0.10.0-preview is the right version.
Having said that, let's put a pin in this until we finalize upon a plan for renaming the namespace and package name

…t into syncInsights

dsgouda

Looks good apart from the one comment

dsgouda · 2018-09-28T22:30:30Z

...ationInsights/DataPlane/Data.ApplicationInsights.Tests/Data.ApplicationInsights.Tests.csproj

@@ -14,7 +14,9 @@
  </ItemGroup>
  <ItemGroup>


Please replace VersionPrefix with Version
Also add <TreatWarningsAsErrors>true</TreatWarningsAsErrors> to the same PropertyGroup

dsgouda

LGTM subject to CIs passing

alexeldeib · 2018-10-02T23:36:03Z

@dsgouda Bump since CI is green?

alexeldeib added 2 commits September 22, 2018 20:41

Revisions for 1.0.0-preview

1daa4b8

Rename to Microsoft.Azure.ApplicationInsights.Query, bump to 1.0.0.0-…

d444b09

…Preview-1

alexeldeib changed the title ~~[App Insights] Sync to swagger, bump to 1.0.0-preview-1~~ [App Insights] Sync to swagger, bump to 1.0.0-preview-1, rename package *breaking changes* Sep 24, 2018

dsgouda added Changes Requested labels Sep 24, 2018

dsgouda suggested changes Sep 24, 2018

View reviewed changes

alexeldeib mentioned this pull request Sep 24, 2018

Recommend path for pulling Application Insights data #4785

Closed

version fix

a5eec25

alexeldeib and others added 4 commits September 25, 2018 15:15

revert version for new package

a8b33e7

Merge branch 'psSdkJson6' into syncInsights

b1b7695

regenerate from master branch

6c082dd

Merge branch 'syncInsights' of github.com:alexeldeib/azure-sdk-for-ne…

e1823e4

…t into syncInsights

dsgouda suggested changes Sep 28, 2018

View reviewed changes

dsgouda removed the spec-not-merged label Sep 28, 2018

Fix version, treatWarningAsError, fix warnings, add docstrings

0a2320c

dsgouda approved these changes Sep 28, 2018

View reviewed changes

dsgouda removed the Changes Requested label Oct 2, 2018

dsgouda merged commit 274e7d0 into Azure:psSdkJson6 Oct 2, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[App Insights] Sync to swagger, bump to 1.0.0-preview-1, rename package breaking changes #4790

[App Insights] Sync to swagger, bump to 1.0.0-preview-1, rename package breaking changes #4790

alexeldeib commented Sep 24, 2018 •

edited

Loading

alexeldeib commented Sep 24, 2018

alexeldeib commented Sep 24, 2018 •

edited

Loading

dsgouda Sep 24, 2018

dsgouda Sep 24, 2018

alexeldeib Sep 24, 2018

dsgouda Sep 24, 2018

dsgouda Sep 24, 2018

alexeldeib Sep 24, 2018

alexeldeib Sep 24, 2018

dsgouda commented Sep 24, 2018

alexeldeib commented Sep 25, 2018

dsgouda commented Sep 25, 2018

dsgouda left a comment

dsgouda Sep 28, 2018

alexeldeib Sep 28, 2018

dsgouda left a comment

alexeldeib commented Oct 2, 2018

[App Insights] Sync to swagger, bump to 1.0.0-preview-1, rename package *breaking changes* #4790

[App Insights] Sync to swagger, bump to 1.0.0-preview-1, rename package *breaking changes* #4790

Conversation

alexeldeib commented Sep 24, 2018 • edited Loading

Description

Azure/azure-rest-api-specs#3771 Azure/azure-rest-api-specs#3084 --> big one Azure/azure-rest-api-specs#3385 Azure/azure-rest-api-specs#3997

alexeldeib commented Sep 24, 2018

alexeldeib commented Sep 24, 2018 • edited Loading

Incomplete

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsgouda commented Sep 24, 2018

alexeldeib commented Sep 25, 2018

dsgouda commented Sep 25, 2018

dsgouda left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dsgouda left a comment

Choose a reason for hiding this comment

alexeldeib commented Oct 2, 2018

[App Insights] Sync to swagger, bump to 1.0.0-preview-1, rename package breaking changes #4790

[App Insights] Sync to swagger, bump to 1.0.0-preview-1, rename package breaking changes #4790

alexeldeib commented Sep 24, 2018 •

edited

Loading

Azure/azure-rest-api-specs#3771
Azure/azure-rest-api-specs#3084 --> big one
Azure/azure-rest-api-specs#3385
Azure/azure-rest-api-specs#3997

alexeldeib commented Sep 24, 2018 •

edited

Loading