Skip to content

[Monitor] Diagnostic settings commands #3668

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
Merged
shahabhijeet merged 8 commits into from
Sep 13, 2017
Merged

[Monitor] Diagnostic settings commands #3668

shahabhijeet merged 8 commits into from
Sep 13, 2017

Conversation

@vcanaa
Copy link
Contributor

@vcanaa vcanaa commented Sep 6, 2017 •
edited
Loading

Description

Renaming the ServiceDiagnosticSettings operation to DiagnosticSettings operation and adding more functionality.

  • The new command will be backward compatible with the existing api /diagnosticSettings/service and will also works with /diagnosticSettings/{name}, where a generic name can be passed in.

  • Renaming ServiceBusRuleId to EventHubAuthorizationRuleId.

  • Accepting EventHubName to be defined

  • Adding List operation to diagnostic settings.

  • Adding operations to list diagnostic settings categories.

Azure/azure-rest-api-specs#1641

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

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.

@shahabhijeet
Copy link
Contributor

shahabhijeet commented Sep 7, 2017

@vcanaa please open this PR once you have sorted out Spec API referenced.
Seems your rest spec was not merged.
Reopen this PR with the update spec PR link

@vcanaa
Copy link
Contributor Author

vcanaa commented Sep 7, 2017

I fixed the reference to the spec. There is no link for me to reopen it.

Please reopen this PR.

@shahabhijeet shahabhijeet reopened this Sep 7, 2017
@shahabhijeet
Copy link
Contributor

shahabhijeet commented Sep 8, 2017

@vcanna please co-ordinate with @gucalder and have a single nuget package published.
We do not want to publish multiple packages within days.

@vcanaa
Copy link
Contributor Author

vcanaa commented Sep 8, 2017

@shahabhijeet - We will not publish multiple packages.

This was marked as Changes Requested, but I don't understand what changes are necessary. What is blocking this to be merged right now?

shahabhijeet
shahabhijeet suggested changes Sep 11, 2017
View reviewed changes
src/SDKs/Monitor/Management.Monitor/Microsoft.Azure.Management.Monitor.csproj
<Description>Microsoft Azure Monitor Library</Description>
<AssemblyName>Microsoft.Azure.Management.Monitor</AssemblyName>
<Version>0.16.1-preview</Version>
<Version>0.17.0-preview</Version>
Copy link
Contributor

@shahabhijeet shahabhijeet Sep 11, 2017

Choose a reason for hiding this comment

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

can you update the release notes
Add < PackageReleaseNotes > and add Release notes.
I would recommend to use < ![CDATA and then format your Release notes

shahabhijeet
shahabhijeet suggested changes Sep 13, 2017
View reviewed changes
src/SDKs/Monitor/Management.Monitor/Microsoft.Azure.Management.Monitor.csproj Outdated
<Version>0.17.0-preview</Version>
<PackageId>Microsoft.Azure.Management.Monitor</PackageId>
<PackageTags>Management.Monitor;Management.Monitoring;metrics;alerts;autoscale;activityLogs;events;operations;logs</PackageTags>
<PackageReleaseNotes>See https://github.com/Azure/azure-sdk-for-net/tree/psSdkJson6/src/SDKs/Monitor/changelog.md for release notes.</PackageReleaseNotes>
Copy link
Contributor

@shahabhijeet shahabhijeet Sep 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

This URL will be of no use once psSdkJson6 branch is deleted
Rather create an aka.ms link for your changelog.md

Copy link
Contributor Author

@vcanaa vcanaa Sep 13, 2017

Choose a reason for hiding this comment

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

Indeed, thanks for catching that. Will update to:
https://aka.ms/azure-sdk-monitor

@shahabhijeet shahabhijeet merged commit 7d5ab4e into Azure:psSdkJson6 Sep 13, 2017
JasonYang-MSFT pushed a commit to JasonYang-MSFT/azure-sdk-for-net that referenced this pull request Dec 8, 2017
@vcanaa @JasonYang-MSFT
[Monitor] Diagnostic settings commands (Azure#3668)
2f37c47 
* Generated code from latest spec

* Updated tests for named diagnostic settings

* [Monitor] Increasing version to 0.17.0-preview

* [Monitor] Adding release notes and syncing with latest spec

* Adding notes about the multi-dim metrics API to the changelog.md file.

* [Monitor] Using an aka.ms link for the changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@shahabhijeet shahabhijeet shahabhijeet approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@vcanaa @shahabhijeet @msftclas @gucalder