-
Notifications
You must be signed in to change notification settings - Fork 283
Create azure_storage_queues crate from azure_storage::queue #493
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
Conversation
I've done some more work on this, and I think it is now looking in better shape. Regarding the
I therefore decided to use this approach, and have created a new I implement a trait to allow a QueueServiceClient to be created from a StorageAccount, so you can write this:
While I was working on the examples, I noticed that all of them had this slightly verbose code to obtain a queue client:
To help ergonomics I implemented
All the tests and examples are now fixed up and working, and the |
Thank you for taking this on @johnbatty! I've recently been around this neck of the woods, so just for your awareness and potential follow-up PRs: |
@thovoll Thanks for the heads-up on the other related issues/PRs. |
Renamed to |
required-features=["queue"] | ||
[[example]] | ||
name="put_message" | ||
required-features=["queue"] |
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.
@johnbatty should these have been added to the new crate?
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.
@thovoll I don't think so. I believe that examples only need to be included in Cargo.toml
if they need additional flags - in this case to add required-features=["queue"]
. In the new crate, this feature flag no longer exists, so we can remove the example sections from Cargo.toml
.
Let me know if you disagree, or I am missing something.
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.
I vaguely remember Cargo complaining when I added examples that were not in Cargo.toml, but it might indeed be because of required-features
. If you can run the examples using Cargo, we're all set I think :)
As per: #483
This is work in progress. I've created the new crate, and the main code builds but there is some cleanup to do, the tests will need updating, and examples need moving/updating.
I'm looking for feedback on whether this broadly the change that you are looking for, and have a few specific questions/issues:
StorageClient
methods referencequeue
functionsI originally moved the entire
queue
module to the new crate, but that caused some build issues/circular dependencies due to methods onStorageClient
that reference functions in thequeue
module, e.g.StorageClient::list_queues(...)
,StorageClient::get_queue_service_properties(...)
. To get the build working I moved these specific requests/responses back into thestorage
crate. I'm looking for suggestions on what the right approach is for these.Possible options:
queue
module remaining withinstorage
for theseStorageClient
level functionsStorageClient
as a new trait implemented in the new crate. I guess a downside of that is that they won't be visible if users look at the documentation forStorageClient
.New crate requires access to
azure_storage
modules and functions that are not currently publicI got the build working by making these public, but am looking for feedback on whether this the right thing to do.
storage::xml
- A small utility module with a single function. Could possible just duplicate this within the new crate.StorageClient
methods:storage_account_client(...)
,queue_url_with_segments(...)
,prepare_request(...)