Skip to content
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

Migrate sdk/storage_blob to azure_core::error scheme #794

Merged
merged 4 commits into from
Jun 14, 2022

Conversation

rickrain
Copy link

This PR migrates the sdk/storage_blob to the new azure_core::error scheme as described in #771.

cataggar
cataggar previously approved these changes Jun 10, 2022
Copy link
Member

@cataggar cataggar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@cataggar cataggar requested a review from johnbatty June 10, 2022 13:59
@johnbatty
Copy link
Contributor

Hi Rick. Many thanks for this - looks great to me, so far...

However, (unfortunately!) there are still quite a few more changes that need to be made for this crate. This crate not only has its own error definitions, but pulls in most of its error definitions from azure_storage. See lib.rs:

pub use azure_storage::{Error, Result};

If you change this to pull in the new-style error definitions from azure_core you'll get a load more compiler errors that will need fixing up.

pub use azure_core::error::{Error, Result};

@rickrain
Copy link
Author

@johnbatty, thanks for pointing that out. I was a little suspicious there weren't more changes. Following your guidance, I'm now seeing 90+ errors that need to be resolved. I'll work on this and ping you and @cataggar to re-review when I'm done.

@bmc-msft bmc-msft requested a review from cataggar June 10, 2022 16:39
@bmc-msft bmc-msft dismissed cataggar’s stale review June 10, 2022 18:57

rickrain commented they have more work to do

@rickrain rickrain changed the title Migrate sdk/storage_blob to azure_core::error scheme WIP: Migrate sdk/storage_blob to azure_core::error scheme Jun 13, 2022
@rickrain
Copy link
Author

Just realizing that the changes here are dependent on the work in #792. I think I have everything done except for what's in sdk/storage_blobs/src/clients/* where the dependency exists. How would you like me to proceed?
@johnbatty, @cataggar

@johnbatty
Copy link
Contributor

@rickrain I just had similar issues working on storage_queues. I decided to get it building independently with the existing (non-converted) storage crate using a map_kind(...) on the non-azure_core error(s) returned by storage. Here's an example from my storage_queues PR:

        self.storage_client
            .queue_url_with_segments(Some(self.queue_name.as_str()).into_iter().chain(segments))
            .map_kind(ErrorKind::DataConversion)

If we can get the higher level storage crates building/merged, then we can merge the storage crate and have a pass through to clean up any unnecessary ErrorKind maps.

@cataggar cataggar removed their request for review June 13, 2022 22:20
@rickrain rickrain changed the title WIP: Migrate sdk/storage_blob to azure_core::error scheme Migrate sdk/storage_blob to azure_core::error scheme Jun 13, 2022
@rickrain
Copy link
Author

@johnbatty, thanks a ton for that pointer. That helped a lot!

@rickrain
Copy link
Author

@johnbatty, @cataggar This PR is ready for review.

@bmc-msft bmc-msft requested a review from cataggar June 14, 2022 00:35
@cataggar cataggar merged commit d95b16d into Azure:main Jun 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants