Fixing BlobClient::uploadWithResponse method to follow the convention and return the response back

[reta]

The convention all API clients are following is that xxxWithResponse methods return the response back. It is not the case for BlobClient::uploadWithResponse, which returns no result. This small fix address this minor deviation. Thank you.

[ghost]

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

[reta]

This is a breaking change (from API semver perspective), is it worth to make it an exception (since the semantic of the API is really not right) or not? Thank you.

[rickle-msft]

@reta First off, thank you for being willing to make a contribution and submit a PR. We appreciate your investment in the project and contribution to the community.

The breaking nature of this change is indeed something that we'll need to discuss and may prevent us from being able to merge it. @alzimmermsft can you please comment on this?

[reta]

Thank you @rickle-msft , totally understandable

[alzimmermsft]

I'll get back on what processes would need to be taken to allow an API breaking change like this, but before I get that information is there any reason why the more comprehensive uploadWithResponse API couldn't be used instead and the one being updated deprecated?

https://github.com/Azure/azure-sdk-for-java/blob/main/sdk/storage/azure-storage-blob/src/main/java/com/azure/storage/blob/BlobClient.java#L283

This API should accept all parameters in the API being modified, except that it is contained in a wrapper class to allow additional parameters in the future without breaking the API.

[reta]

@alzimmermsft thank you for clarification, this is exactly what I am doing (using the other version), so no issues here. Just from the API perspective, it looks inconsistent and does not follow its own conventions (the response is expected but I was taken by surprise). This is the only motivation, not a blocking issue, thank you.

[alzimmermsft]

@rickle-msft, I'm leaning towards marking the API as deprecated and redirecting usage to the newer, more versatile, API which has the response as well.

[rickle-msft]

That works for me.

@alzimmermsft @reta Would it be ok to close this PR for now and I can open one to deprecate this api and point towards the newer one after we release this week? That way it can go out in our next preview? I'll create a tracking issue so it doesn't get lost?

@reta I will tag you in the new PR when I open it, and if you don't see anything in a couple weeks and feel so inclined, please don't hesitate to tag me here and ask for an update

[reta]

Thanks a lot @rachel-msft and @alzimmermsft , certainly works for me!

[rickle-msft]

Created the issue. #24108