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

Update to Release 3.5 standard (pre GA) #5370

Merged
merged 24 commits into from
Mar 20, 2019
Merged

Conversation

leonardbf
Copy link
Contributor

Still in preview - breaking changes are not considered applicable.

Changes:

  • Inclusion of status code (due to sync).
  • Inclusion of account and volume property (AD and export policy)
  • Inclusion of missing capacityPool patch properties
  • ServiceLevel name change
  • Changed resourceGroup to resourceGroupName (correct naming for CLI features)
    (internal ticket NFSAAS-1875)

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.

leonardbf and others added 21 commits November 20, 2018 08:04
NFSAAS-1505 minor update and validation fixes
Put (and patch) for snapshots.
Service level definition changes.
NFSAAS-1505 correct resource reference and limits
Incorrect pool resource reference
Name elements of property not required
Correction to return status code
NFSAAS-1505 resource and property changes
NFSAAS-1644 remove mt get and add put status code
NFSAAS-1644 remove mt get add status for put
@azuresdkci
Copy link
Contributor

Can one of the admins verify this patch?

@AutorestCI
Copy link

AutorestCI commented Mar 13, 2019

Automation for azure-sdk-for-ruby

Nothing to generate for azure-sdk-for-ruby

@AutorestCI
Copy link

AutorestCI commented Mar 13, 2019

Automation for azure-sdk-for-python

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

@AutorestCI
Copy link

AutorestCI commented Mar 13, 2019

Automation for azure-sdk-for-js

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-js#1665

@AutorestCI
Copy link

AutorestCI commented Mar 13, 2019

Automation for azure-sdk-for-node

Nothing to generate for azure-sdk-for-node

@AutorestCI
Copy link

AutorestCI commented Mar 13, 2019

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@AutorestCI
Copy link

AutorestCI commented Mar 13, 2019

Automation for azure-sdk-for-go

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

@adxsdknet
Copy link

adxsdknet commented Mar 13, 2019

Automation for azure-sdk-for-net

A PR has been created for you:
Azure/azure-sdk-for-net#5431
.NET SDK Commits:
adxsdknet/azure-sdk-for-net@526dc91
adxsdknet/azure-sdk-for-net@af1c3f0
adxsdknet/azure-sdk-for-net@0bf4c9b

adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 13, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5370'
REST Spec PR Author 'leonardbf'
REST Spec PR Last commit
* NFSAAS-1875 Update to R3.5 Standard

* NFSAAS-1875 update to R3.5 standard
adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 13, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5370'
REST Spec PR Author 'leonardbf'
REST Spec PR Last commit
@veronicagg veronicagg assigned kpajdzik and unassigned veronicagg Mar 13, 2019
@veronicagg
Copy link
Contributor

@kpajdzik would you take this one? thanks!

@kpajdzik
Copy link
Contributor

@veronicagg I will :)

"properties": {
"activeDirectoryId": {
"type": "string",
"description": "Id of the active drectory"
Copy link
Contributor

Choose a reason for hiding this comment

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

drectory -> directory
Maybe also capitalize too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

},
"status": {
"type": "string",
"description": "Status of the active drectory"
Copy link
Contributor

Choose a reason for hiding this comment

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

drectory -> directory
Maybe also capitalize too

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

},
"allowedClients": {
"type": "string",
"description": "Client ingress specification as comma seperated string with IPv4 CIDRs, IPv4 host addresses and host names"
Copy link
Contributor

Choose a reason for hiding this comment

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

seperated -> separated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -1627,6 +1771,12 @@
"type": "string",
"example": "255.255.255.0"
},
"smbServerFqdn": {
"title": "smbServerFQDN",
"description": "The SMB server's Fully Qualified Doman Name, FQDN",
Copy link
Contributor

Choose a reason for hiding this comment

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

Doman -> Domain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

* NFSAAS-1875 Update to R3.5 Standard

* NFSAAS-1875 update to R3.5 standard

* NFSAAS-1875 updated from review comments
adxsdknet added a commit to adxsdknet/azure-sdk-for-net that referenced this pull request Mar 13, 2019
REST Spec PR 'Azure/azure-rest-api-specs#5370'
REST Spec PR Author 'leonardbf'
REST Spec PR Last commit
Merging to fork from azure master
@kpajdzik
Copy link
Contributor

If you haven't taken a look at our new onboarding experience at OpenAPIHub, it's a convenient way to create your PR when you're copying from an existing API version or when you're editing your existing specs. If you have any feedback or questions, feel free to use the feedback button on top of the site for help. Thanks!

@kpajdzik
Copy link
Contributor

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@anuchandy Could you please take a look?

"type": "string",
"description": "Username of Active Directory domain administrator"
},
"password": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you elaborate on this parameter? Is this secure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's credentials for the AD admin at the lower level. The RP only passes this through to the lower level. It is transmitted via https to the RP. But yes quite correct, the security is in question and there is an internal ticket to address this for the next release (pre-GA). Of course this swagger only reflects this implementation.

"type": "boolean",
"description": "Read only access"
},
"unixReadWrite": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it prefixed with "unix"? Should this be included in description as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a consequence of a lower level API that the RP calls. This value is simply passed through. But it is because it only applies to the NFS permissions. It doesn’t do anything useful for NTFS.

@leonardbf
Copy link
Contributor Author

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@anuchandy Could you please take a look?

Just a clarification - is Java SDK something that Microsoft will auto generate? Is it one of those handled from within the Devx OpenAPI hub for instance?

@kpajdzik
Copy link
Contributor

Automation for azure-sdk-for-java

Nothing to generate for azure-sdk-for-java

@anuchandy Could you please take a look?

Just a clarification - is Java SDK something that Microsoft will auto generate? Is it one of those handled from within the Devx OpenAPI hub for instance?

Yes, it is the one handled by OpenAPI portal. It included Java, Node.js (soon deprecated so no new SDKs), TypeScript, Ruby and Python.

@leonardbf
Copy link
Contributor Author

I've provided answers for the comments but this PR is still open. Can someone either respond if there are further issues or merge please.
Thanks.

@leonardbf
Copy link
Contributor Author

Since this is approved, is there any chance it could be merged? Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants