Skip to content

Kusto: Added more features such as: database operations, check cluste… #3895

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

Merged
merged 18 commits into from
Sep 20, 2018
Merged

Kusto: Added more features such as: database operations, check cluste… #3895

merged 18 commits into from
Sep 20, 2018

Conversation

liatbezalel
Copy link
Contributor

@liatbezalel liatbezalel commented Sep 13, 2018

…r/database name availability, cluster - start/stop, event hub data connection, list skus, list resource skus. Added new api version (features are identical to the older version).

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

…r/database name availability, cluster - start/stop, event hub data connection, list skus, list resource skus
@AutorestCI
Copy link

AutorestCI commented Sep 13, 2018

Automation for azure-sdk-for-go

Nothing to generate for azure-sdk-for-go

@AutorestCI
Copy link

AutorestCI commented Sep 13, 2018

Automation for azure-sdk-for-python

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

@AutorestCI
Copy link

AutorestCI commented Sep 13, 2018

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Sep 13, 2018

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@AutorestCI
Copy link

AutorestCI commented Sep 13, 2018

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@annatisch
Copy link
Member

This PR is a continuation of #3595 which has already received ARM sign-off.

@annatisch
Copy link
Member

Thanks @liatbezalel - could you please fix the CI? There seems to be missing references:

Uncaught Error: /home/travis/build/Azure/azure-rest-api-specs/specification/azure-kusto/resource-manager/Microsoft.Kusto/2017-09-07-privatepreview/kusto.json has references that cannot be resolved. They are as follows:
'Error opening file "/home/travis/build/Azure/azure-rest-api-specs/specification/azure-kusto/resource-manager/Microsoft.Kusto/2017-09-07-privatepreview/examples/KustDataconnectionValidation.json" \nENOENT: no such file or directory, open \'/home/travis/build/Azure/azure-rest-api-specs/specification/azure-kusto/resource-manager/Microsoft.Kusto/2017-09-07-privatepreview/examples/KustDataconnectionValidation.json\''

@annatisch
Copy link
Member

Also @liatbezalel - I see that there are now two new specs in this PR - preview and privatepreview.
What's the difference between them and does the 2nd spec also require ARM sign-off?

@liatbezalel
Copy link
Contributor Author

@annatisch they are both identical, so there is no need for ARM sign-off. (Also fixed the missing reference).

@annatisch
Copy link
Member

Thanks @liatbezalel!
There are still some linter errors (ARM violations):
https://github.com/Azure/azure-rest-api-specs/pull/3895/checks?check_run_id=17718124#user-content-ARM-Errors

There's also a large number of model validation errors:
https://travis-ci.org/Azure/azure-rest-api-specs/jobs/429432043

@liatbezalel
Copy link
Contributor Author

@annatisch everything is green now :)

@annatisch
Copy link
Member

Thanks @liatbezalel - I will try to review this afternoon

@liatbezalel
Copy link
Contributor Author

@annatisch ping

Copy link
Member

@annatisch annatisch left a comment

Choose a reason for hiding this comment

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

Looks pretty good - just a few minor things.
In addition to my review comments - a couple of general things:

  • Please check the descriptions to capitalize abbreviations (e.g. URI, GUID, ID etc) and using capital letters at the start and period at the end.
  • Please move the spec files into "stable" or "preview" subfolders. Please take a look at other service spec directory structure to see how this is done.

Thanks! :)

@AutorestCI
Copy link

AutorestCI commented Sep 19, 2018

Automation for azure-sdk-for-js

Nothing to generate for azure-sdk-for-js

@liatbezalel
Copy link
Contributor Author

thanks @annatisch, resolved all your comments.

@annatisch
Copy link
Member

Thanks @liatbezalel!
It looks like you will need to update the relative references to the common types since the change in directory structure, e.g.:

"$ref": "../../../../common-types/resource-management/v1/types.json#/definitions/TrackedResource"

@liatbezalel
Copy link
Contributor Author

@annatisch fixed :)

@annatisch
Copy link
Member

Thanks @liatbezalel - it looks good.
Two points:

  • I'm really sorry but I think I made a mistake in my feedback - looking at it more closely, I think this operation should be called "Clusters_ListSkusByResource rather than "Clusters_ListSkusByResourceGroup" as it takes the cluster resource name as a parameters as well as the group.
  • Are you aware that there will be breaking changes in the Python SDK? While I'm not certain, there may be breaking changes in other language SDKs as well - are you okay with a major version bump for these new features?

Thanks!

@liatbezalel
Copy link
Contributor Author

liatbezalel commented Sep 20, 2018

@annatisch

  1. Changed.
  2. We are ok with that, it won't break anything.

@annatisch annatisch merged commit b3d4480 into Azure:master Sep 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants