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

Add Swagger documention for UpgradeDataWarehouse API #2426

Closed
wants to merge 7 commits into from
Closed

Add Swagger documention for UpgradeDataWarehouse API #2426

wants to merge 7 commits into from

Conversation

vsporeddy
Copy link

@vsporeddy vsporeddy commented Feb 6, 2018

UpgradeDataWarehouse allows users to perform maintenance updates on their SQL Data Warehouse instances at their own convenience. This change adds the API specifications for this feature.

Venkata Srikar Poreddy added 3 commits February 5, 2018 16:11
UpgradeDataWarehouse allows users to perform maintenance updates on their SQL Data Warehouse instances at their own convenience. This change adds the API specifications for this feature.
@AutorestCI
Copy link

AutorestCI commented Feb 6, 2018

Automation for azure-sdk-for-python

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-python#1950

@azuresdkciprbot
Copy link

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/sql/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 0
After the PR: Warning(s): 0 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@AutorestCI
Copy link

Swagger to SDK encountered a Subprocess error: (Azure/azure-sdk-for-go)

Command: profileBuilder -s preview -name preview
Finished with return code 127
and output:

/bin/sh: 1: profileBuilder: not found

Fix missing curly brace in databases.json
@azuresdkciprbot
Copy link

Hi There,

I am the AutoRest Linter Azure bot. I am here to help. My task is to analyze the situation from the AutoRest linter perspective. Please review the below analysis result:

File: specification/sql/resource-manager/readme.md
Before the PR: Warning(s): 0 Error(s): 0
After the PR: Warning(s): 0 Error(s): 0

AutoRest Linter Guidelines | AutoRest Linter Issues | Send feedback

Thanks for your co-operation.

@AutorestCI
Copy link

Swagger to SDK encountered a Subprocess error: (Azure/azure-sdk-for-go)

Command: profileBuilder -s preview -name preview
Finished with return code 127
and output:

/bin/sh: 1: profileBuilder: not found

@marstr
Copy link
Member

marstr commented Feb 9, 2018

@vsporeddy these changes are to an API Version that is in preview. For that reason, you can opt to skip some of the normal review process. However, it would also mean that SDK owners may not generate libraries to target this service.

Is this API Version correctly sorted?

@vsporeddy
Copy link
Author

@marstr Yes, this is for a preview API version. However I want it to be available in .NET SDK and Azure PowerShell, is that possible?

@marstr
Copy link
Member

marstr commented Feb 13, 2018

I see, sorry for the confusion! So, we should classify this as belonging in the stable folder, because it is a Swagger that will be built upon by customers. Please make that update, and we'll start the ARM review, etc.

@marstr marstr added the WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required label Feb 13, 2018
@vsporeddy
Copy link
Author

Oh no, I meant I was going to make it available in .NET SDK/Azure PowerShell myself. It is still only for 2017-03-01-preview.

@marstr
Copy link
Member

marstr commented Feb 14, 2018

@vsporeddy says:
I meant I was going to make it available in .NET SDK/Azure PowerShell myself

Ah just for a private preview, or with the intention of shipping to customers? I think there are two things getting confused here:

  1. Whether the API Version is in preview or not (should be denoted by the "-preview" suffix), and speaks to the SLA and long-term availability of that API Version.
  2. Whether or not the Swagger is ready for publication to customers (outside of private preview). This is indicated by whether the API Version is in the preview or stable folder in this repository.

The two aspects are independent of one another. Clearly the API Version is under a more lenient SLA, but should it be considered ready for customers to play with?

@ravbhatnagar ravbhatnagar added ARMSignedOff <valid label in PR review process>add this label when ARM approve updates after review and removed WaitForARMFeedback <valid label in PR review process> add this label when ARM review is required labels Feb 15, 2018
@ravbhatnagar
Copy link
Contributor

Looks good.

@marstr
Copy link
Member

marstr commented Feb 15, 2018

KK, well all moot at this point! @vsporeddy are you ready for me to merge this in?

@AutorestCI
Copy link

AutorestCI commented Feb 16, 2018

Automation for azure-sdk-for-go

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-sdk-for-go#1258

@@ -480,6 +480,52 @@
}
}
}
},
"/subscriptions/{subscriptionId}/resourceGroups/{resourceGroupName}/providers/Microsoft.Sql/servers/{serverName}/databases/{databaseName}/upgradeDataWarehouse": {
Copy link
Contributor

Choose a reason for hiding this comment

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

missing x-ms-long-running-operation?

Copy link
Contributor

@jaredmoo jaredmoo left a comment

Choose a reason for hiding this comment

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

missing x-ms-long-running-operation

@marstr
Copy link
Member

marstr commented Feb 23, 2018

Just to update this thread, we're currently having some offline conversations about the preview folder. We should respond more adequately tomorrow with more information.

@marstr
Copy link
Member

marstr commented Feb 27, 2018

Sorry for the delay on this.

After chatting about it internally with @salameer, @johanste, and @lmazuel it seems like some of my statements above had some misinformation. It sounds like we're still committed to publishing bits in the preview folder as public-previews. The only real difference between the stable and preview folders is that SDKs are required to somehow convey to consumers of SDKs generated from Swaggers in the preivew folder that breaking changes may come. Different SDKs will interpret that differently. Python for instance, will release PyPI packages with a suffix to indicate preview status..

@lmazuel
Copy link
Member

lmazuel commented Mar 9, 2018

@AutorestCI rebuild azure-sdk-for-python

@marstr
Copy link
Member

marstr commented Mar 9, 2018

@AutorestCI rebuild azure-sdk-for-go

@marstr
Copy link
Member

marstr commented Mar 12, 2018

Howdy @vsporeddy, with the context that I layout above, what is the current status of this PR? Is it ready to be in the master branch in the preview folder?

@marstr
Copy link
Member

marstr commented Mar 14, 2018

Closing this PR as stale. If these changes are still relevant, feel free to have it re-opened or create a new PR. One caveat, I'll be OOF starting Friday the 16th, so if it is re-opened it will need to be reassigned.

@marstr marstr closed this Mar 14, 2018
@AutorestCI
Copy link

AutorestCI commented Mar 14, 2018

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

@AutorestCI
Copy link

AutorestCI commented Mar 14, 2018

Automation for azure-libraries-for-java

A PR has been created for you based on this PR content.

Once this PR will be merged, content will be added to your service PR:
Azure/azure-libraries-for-java#292

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.

7 participants