Azure / azure-sdk-for-rust

This repository is for active development of the *unofficial* Azure SDK for Rust. This repository is *not* supported by the Azure SDK team.
MIT License
692 stars 240 forks source link

split up azure_storage #483

Closed cataggar closed 2 years ago

cataggar commented 2 years ago

Per https://azure.github.io/azure-sdk/general_design.html#namespaces, please split up storage into crates:

cataggar commented 2 years ago

Previous crate naming details are in #304.

Not important, but good to know that the crate names match product marketing URLs well:

.NET SDK packages for reference:

johnbatty commented 2 years ago

I'm going to have a go at this, starting with azure_messaging_queues.

johnbatty commented 2 years ago

A couple of questions about naming/structure after looking at this:

johnbatty commented 2 years ago

I've done azure_messaging_queues, moving on to azure_storage_blobs...

thovoll commented 2 years ago

A couple of questions about naming/structure after looking at this:

  • azure_messaging_queues naming: I understand the rationale that queues logically belong in the messaging namespace rather than the storage namespace. However, all the other Azure SDKs I have looked at have it in the storage namespace. Are we happy that the Rust naming will not be consistent with the other SDKs?
  • There is some common storage function (e.g. StorageClient). I therefore presume that the azure_storage crate should remain to contain this common code, even when the other function is split out into separate crates. Do you agree?

@JeffreyRichter or @heaths might be able to shed some light on the first question.

On the second question, I propose we eliminate the StorageAccountClient and the StorageClient, just like in the Java and .Net SDKs. Keep in mind that I have not looked at the other SDKs.

JeffreyRichter commented 2 years ago

I spoke to the other language architects about this. It turns out that putting queues in Storage was a mistake; they all meant to put it in Messaging. Basically the 1st language did Storage and the other languages just followed this. We can keep Storage for consistency.

We tend to favor replication code over having a common dependency for a few reasons:

thovoll commented 2 years ago

Thanks @JeffreyRichter. To be clear, you are saying that we should keep queues under storage correct?

I totally agree with not over-using common dependencies. More often than not, code duplication is trivial and way cheaper to maintain than a common dependency (as long as we are not duplicating any sophisticated algorithms).

JeffreyRichter commented 2 years ago

It's a hard call sometimes whether to maintain consistency across languages when you learn from a mistake and know something could be better. For true bugs and patterns that cause customer pain, we know we'd fix it in another language if we can and break language consistency.

In this case, it's just naming (not a real bug) and so let's keep queues under Storage instead of Messaging.

johnbatty commented 2 years ago

Thanks @JeffreyRichter - I'll update my PR for the queues crate to call it azure_storage_queues.

johnbatty commented 2 years ago

I'm moving on to azure_storage_datalake next...

Just to confirm - is it definitely azure_storage_datalake, rather than azure_storage_data_lake?

johnbatty commented 2 years ago

Next up... azure_data_tables...

cataggar commented 2 years ago

Just to confirm - is it definitely azure_storage_datalake, rather than azure_storage_data_lake?

Confirmed. Thanks @johnbatty