azure-rest-api-specs icon indicating copy to clipboard operation
azure-rest-api-specs copied to clipboard

Blob Storage - Remove regex pattern constraint from blob name parameter

Open okofish opened this issue 3 years ago • 38 comments

Changelog

Add a changelog entry for this PR by answering the following questions:

  1. What's the purpose of the update?
    • [ ] new service onboarding
    • [ ] new API version
    • [ ] update existing version for new feature
    • [ ] update existing version to fix swagger quality issue in s360
    • [x] Other, please clarify
      • Remove over-restrictive parameter constraint. Blob service documentation says that "A blob name can contain any combination of characters". The removed constraint restricts blob names to alphanumeric characters with some token punctuation.
  2. When are you targeting to deploy the new service/feature to public regions? Please provide the date or, if the date is not yet available, the month.
    • N/A
  3. When do you expect to publish the swagger? Please provide date or, the the date is not yet available, the month.
    • N/A (I think?)
  4. If updating an existing version, please select the specific langauge SDKs and CLIs that must be refreshed after the swagger is published.
    • [ ] SDK of .NET (need service team to ensure code readiness)
    • [ ] SDK of Python
    • [ ] SDK of Java
    • [ ] SDK of Js
    • [ ] SDK of Go
    • [ ] PowerShell
    • [ ] CLI
    • [ ] Terraform
    • [ ] No refresh required for updates in this PR

I'm unsure of how to answer the above question. Since this change removes a constraint, should all the SDKs be regenerated?

Contribution checklist:

  • [x] I commit to follow the Breaking Change Policy of "no breaking changes"
  • [ ] I have reviewed the documentation for the workflow.
    • N/A - I do not have permissions to access the linked page.
  • [ ] Validation tools were run on swagger spec(s) and errors have all been fixed in this PR. How to fix?
    • N/A - I do not have permissions to access the linked page. I assume CI will catch validation errors.

If any further question about AME onboarding or validation tools, please view the FAQ.

ARM API Review Checklist

Applicability: :warning:

If your changes encompass only the following scenarios, you should SKIP this section, as these scenarios do not require ARM review.

  • Change to data plane APIs
  • Adding new properties
  • All removals

Otherwise your PR may be subject to ARM review requirements. Complete the following:

  • [ ] Check this box if any of the following apply to the PR so that label “WaitForARMFeedback” will be added automatically to begin ARM API Review. Failure to comply may result in delays to the manifest.

    • Adding a new service
    • Adding new API(s)
    • Adding a new API version -[ ] To review changes efficiently, ensure you copy the existing version into the new directory structure for first commit and then push new changes, including version updates, in separate commits.
  • [ ] Ensure you've reviewed following guidelines including ARM resource provider contract and REST guidelines. Estimated time (4 hours). This is required before you can request review from ARM API Review board.

  • [ ] If you are blocked on ARM review and want to get the PR merged with urgency, please get the ARM oncall for reviews (RP Manifest Approvers team under Azure Resource Manager service) from IcM and reach out to them.

Breaking Change Review Checklist

If any of the following scenarios apply to the PR, request approval from the Breaking Change Review Board as defined in the Breaking Change Policy.

  • [ ] Removing API(s) in a stable version
  • [ ] Removing properties in a stable version
  • [ ] Removing API version(s) in a stable version
  • [ ] Updating API in a stable or public preview version with Breaking Change Validation errors
  • [ ] Updating API(s) in public preview over 1 year (refer to Retirement of Previews)

Action: to initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki.

Please follow the link to find more details on PR review process.

okofish avatar Sep 17 '21 17:09 okofish

Hi, @okofish Thanks for your PR. I am workflow bot for review process. Here are some small tips.

  • Please ensure to do self-check against checklists in first PR comment.
  • PR assignee is the person auto-assigned and responsible for your current PR reviewing and merging.
  • For specs comparison cross API versions, Use API Specs Comparison Report Generator
  • If there is CI failure(s), to fix CI error(s) is mandatory for PR merging; or you need to provide justification in PR comment for explanation. How to fix?
  • Any feedback about review process or workflow bot, pls contact swagger and tools team. [email protected]

    [Call for Action] To better understand Azure service dev/test scenario, and support Azure service developer better on Swagger and REST API related tests in early phase, please help to fill in with this survey https://aka.ms/SurveyForEarlyPhase. It will take 5 to 10 minutes. If you already complete survey, please neglect this comment. Thanks.

    Swagger pipeline restarted successfully, please wait for status update in this comment.

    Swagger pipeline restarted successfully, please wait for status update in this comment.

    Thank you for your contribution okofish! We will review the pull request and get back to you soon.

    ghost avatar Sep 17 '21 17:09 ghost

    Hi @okofish, one or multiple breaking change(s) is detected in your PR. Please check out the breaking change(s), and provide business justification in the PR comment and @ PR assignee why you must have these change(s), and how external customer impact can be mitigated. Please ensure to follow breaking change policy to request breaking change review and approval before proceeding swagger PR review. Action: To initiate an evaluation of the breaking change, create a new intake using the template for breaking changes. Addition details on the process and office hours are on the Breaking change Wiki. If you want to know the production traffic statistic, please see ARM Traffic statistic. If you think it is false positive breaking change, please provide the reasons in the PR comment, report to Swagger Tooling Team via https://aka.ms/swaggerfeedback.

    Hmm, I'm not sure how removing a constraint constitutes a breaking change. I cannot access any of the links in the bot's comment to the breaking change policy or anything else. @anuchandy, can you please let me know how to proceed from here? Thank you.

    Edit: I guess it could be a breaking change if the schema having the constraint removed were itself a not constraint for another schema. I don't think any other schema uses "Blob" as a not constraint.

    okofish avatar Sep 17 '21 18:09 okofish

    Hi, @okofish. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    ghost avatar Oct 10 '21 16:10 ghost

    I don't think I have permission to remove the no-recent-activity label 😕

    @anuchandy @seanmcc-msft @kasobol-msft Could one of you please take a look at this and let me know how to proceed? Thank you!

    okofish avatar Oct 10 '21 17:10 okofish

    Hi, @okofish. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    ghost avatar Nov 07 '21 16:11 ghost

    Commenting to mark as unstale.

    @anuchandy @seanmcc-msft @kasobol-msft Could one of you please take a look at this and let me know how to proceed? Thank you in advance!

    okofish avatar Nov 07 '21 17:11 okofish

    Hi, @okofish. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    ghost avatar Nov 28 '21 16:11 ghost

    Commenting to mark as unstale.

    okofish avatar Nov 28 '21 21:11 okofish

    Hi, @okofish. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    ghost avatar Dec 19 '21 16:12 ghost

    Hi, @okofish. The PR will be closed since the PR has no update for 28 days. If you still need the PR review to proceed, please reopen it and @ mention PR assignee.

    ghost avatar Jan 03 '22 05:01 ghost

    I have to say I'm disappointed by the complete lack of maintainer attention to a good-faith PR on this open-source repo. A little bit unprofessional :/ Oh well...

    okofish avatar Jan 04 '22 13:01 okofish

    @okofish / Jesse, I am not sure if removing constraints from the blobname is the right thing to do. Can you please provide a justification for this change? @seanmcc-msft to look when he is back.

    amishra-dev avatar Jan 04 '22 18:01 amishra-dev

    Hi @amishra-dev, thanks for your response. My justification is that the blob name constraint is too strict. The current regex admits only alphanumeric characters, and only blob names following a strict filename-like syntax. It does not match, for example, $FooBar or abc/def.tar.gz, both of which are valid blob names.

    The blob service documentation says that "A blob name can contain any combination of characters".

    okofish avatar Jan 04 '22 18:01 okofish

    Hi, @okofish. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    ghost avatar Jan 30 '22 16:01 ghost

    Commenting to mark as unstale.

    okofish avatar Jan 30 '22 23:01 okofish

    Hi, @okofish. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    ghost avatar Feb 20 '22 16:02 ghost

    Commenting to mark as unstale.

    okofish avatar Feb 24 '22 17:02 okofish

    Hi, @okofish. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    ghost avatar Mar 13 '22 16:03 ghost

    Commenting to mark as unstale.

    okofish avatar Mar 13 '22 23:03 okofish

    Hi, @okofish. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    ghost avatar Apr 03 '22 16:04 ghost

    Commenting to mark as unstale.

    okofish avatar Apr 03 '22 22:04 okofish

    Hi, @okofish. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    dohlenaugur avatar Apr 24 '22 16:04 dohlenaugur

    Commenting to mark as unstale.

    okofish avatar Apr 24 '22 16:04 okofish

    Hi, @okofish. Your PR has no update for 14 days and it is marked as stale PR. If no further update for over 14 days, the bot will close the PR. If you want to refresh the PR, please remove no-recent-activity label.

    ramonfontes avatar May 15 '22 16:05 ramonfontes

    Commenting to mark as unstale.

    okofish avatar May 15 '22 22:05 okofish