-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Change storagecache name max length from 31 to 80 #8606
Conversation
You don't have permission to trigger SDK Automation. |
Can one of the admins verify this patch? |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
azure-sdk-for-net - Release
|
azure-sdk-for-java - Release
|
azure-sdk-for-js - Release
|
azure-sdk-for-python - Release
|
azure-sdk-for-go - Release
|
hello @jsmartt , becasue of the new CI rule, field with |
@njuCZ, I think I found where to add the |
Great, thanks @romahamu. It looks like that PR has a new |
@jsmartt May I know the status of current PR? It seems long time no update and there is still a Semantic CI error needs to be addressed. If this PR is outdate, I am going to close it |
@njuCZ, yes, this is still valid. I can't speak to your testing process though. I see no reason that these changes would break any tests, so maybe there's an issue with the testing process, not this PR? |
9b79acc
to
d9c9201
Compare
@njuCZ, I've tried a few different ways to fix the semantic check, but I can't figure it out, and I don't think it's something that this PR introduces, but rather an existing issue with the file, or a change to the testing process after this file was created. Feel free to add a commit to fix those issues if you'd like, but I don't have the time to figure out the intricacies of the expected format. That gets a bit outside of the purpose of this PR, which is to fix a different issue that is affecting validations in the SDKs. |
@jsmartt just having a look at the pipeline, it says |
@njuCZ, can you provide a line number and the code to do so? I've tried a few different times to no avail. |
@jsmartt we could find error message here: https://dev.azure.com/azure-sdk/public/_build/results?buildId=457263&view=logs&jobId=95dbed5b-845d-57ba-33d3-f6c5e69afcf4&j=95dbed5b-845d-57ba-33d3-f6c5e69afcf4&t=44f1510e-a822-56b0-2e0b-241e0a387c6a which says
please check it's not related with your PR, but the problem of the original file. to proceed with this PR, we need to fix it. |
@njuCZ, I've added a commit, but it's clearly not the correct fix. As I mentioned above, I've tried a few different ways to fix the semantic check, but I can't figure it out. Feel free to add a commit to fix those issues if you'd like, but I don't have the time to figure out the intricacies of the expected format. If you would like me to add fixes to this PR for other issues, please include the code you'd like me to add and the line number to add it to. This all is outside of the purpose of this PR, which is to fix a different issue that is affecting validations in the SDKs. |
@jsmartt please follow this example to mark it as required: https://github.com/azure/azure-rest-api-specs/blob/master/specification/cdn/resource-manager/Microsoft.Cdn/stable/2019-06-15/cdn.json#L2284 |
db16136
to
4461db4
Compare
4461db4
to
360699b
Compare
@njuCZ, ok, I've added the requested change, and the semantic check is passing now. I had to move the location of the discriminator because it was in the wrong location; this places it in the same location as the updated spec. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
azure-sdk-for-python-track2 - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
Trenton Generation - Release
No readme.md specification configuration files were found that are associated with the files modified in this pull request, or swagger_to_sdk section in readme.md is not configured
|
Azure CLI Extension Generation - Release
|
* Change storagecache name max length from 31 to 80 * Require storagetarget param to create storagetarget for storagecache * Require StorageCache discriminator: targetType * Undo require StorageCache discriminator: targetType * Require StorageCache StorageTarget discriminator: targetType
Related to Azure/azure-sdk-for-ruby#2759
This updates the storagecache name regex to support a maximum length of 80, not 31. This is consistent with the Azure console validation messages, and with my own experience creating caches with names up to 80 characters long. This confusion may have originated from cache storage target name restrictions of 31 characters, which is correct.
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:
ARM API Review Checklist
Failure to comply may result in delays for manifest application. Note this does not apply to data plane APIs.
Please follow the link to find more details on API review process.