-
Notifications
You must be signed in to change notification settings - Fork 276
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
Fix find blob by tag #1622
Fix find blob by tag #1622
Conversation
There were 2 issues in here, one is that the API version got updated recently and that caused the response to be different than the one expected so querying blobs by tag wasn't working. Another item was when we add multiple tags to the where expression, that was failing because of the way the url was being encoded and that doesn't seem to be accepted by Azure REST API
@seanmcc-msft is this a known breaking change to the format? |
I took the chance to hook up the MaxResults into FindBlobsByTagsBuilder as it wasn't doing anything, removed the next_marker on the input and change it to Marker to match the |
@seanmcc-msft is there anyone on your team that can review this? It's mostly service-related code. |
@jaschrep-msft @vincenttran-msft @jalauzon-msft please take a look. |
fn make_url_compatible_with_api(url: &mut Url) { | ||
let compatible_query = url.query().map(|q| q.replace("+", "%20")); | ||
|
||
url.set_query(compatible_query.as_deref()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
General note, requiring %20
instead of +
as the proper encoding for a space character is service-wide. Any URL encoding done by the library for storage requests needs to do this. Probably beyond the scope of this PR, but good future reference.
Updates the requirements on [base64](https://github.com/marshallpierce/rust-base64) to permit the latest version. - [Changelog](https://github.com/marshallpierce/rust-base64/blob/master/RELEASE-NOTES.md) - [Commits](marshallpierce/rust-base64@v0.21.0...v0.22.0) --- updated-dependencies: - dependency-name: base64 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
dcc3aea
to
1b750e2
Compare
|
Hi @rafamerlin. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days. |
Hi @rafamerlin. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing |
There were 2 issues in here, one is that the API version got updated recently and that caused the response to be different than the one expected so querying blobs by tag wasn't working. I believe the version that the current
main
code has is the response for when we query withx-ms-version: 2019-12-12
, however, we're now sending our requests withx-ms-version: 2022-11-02
which caused this to completely break.Another item was when we add multiple tags to the where expression, that was failing because of the way the URL was being encoded and that doesn't seem to be accepted by Azure REST API.
I've been using a workaround branch that was still using the old API for quite a while without any issues with this URL fix, related to this issue I raised a while ago #1284
The error seems to have changed on the new API version as without the
make_url_compatible_with_api
if we query anything with more than 1 tag in the filter we get this error:One question I have regarding the extra structs I had to add to have the response matching the new format of the XML for the tag content, do we need that at all? I was wondering if we could just drop the tag content as it doesn't seem like it's being used anywhere. Happy to keep it though in case there's any intention of using it in the future or if we want it for information purposes.