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

Redesign crates, add Azure support #79

Merged
merged 23 commits into from
Jan 15, 2024
Merged

Redesign crates, add Azure support #79

merged 23 commits into from
Jan 15, 2024

Conversation

auguwu
Copy link
Member

@auguwu auguwu commented Nov 26, 2023

This crate does a whole lot of re-designing for the long run.

  • Removed remi_core as remi should serve the same purpose
  • Restructure arguments to methods that rely on Strings or &[u8] to use an impl of Into<T>.
  • Having methods "panic" to seal a mut reference is stupid and I wish I never did this! This uses mut self instead of &mut self.

This crate also adds experimental support for Azure Blob Storage via azure_blob_storage crate and Google Cloud Storage via google-cloud-storage crate.

@auguwu auguwu requested a review from IceeMC as a code owner November 26, 2023 06:49
@auguwu auguwu marked this pull request as draft November 26, 2023 06:53
@auguwu auguwu self-assigned this Nov 26, 2023
@auguwu auguwu added documentation Improvements or additions to documentation enhancement New feature or request labels Nov 26, 2023
@auguwu auguwu marked this pull request as ready for review January 6, 2024 08:27
@auguwu auguwu removed the request for review from IceeMC January 15, 2024 01:46
Copy link
Contributor

@spotlightishere spotlightishere left a comment

Choose a reason for hiding this comment

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

This is an extremely large pull request, so I can't quite do an in-depth review! However, from looking at the general scope of changes, this seems like a promising direction - and certainly assists with moving a lot of the AWS S3 SDK specific things out!

I left only quick comments asking to please make TODOs in source also become GitHub issues - would appreciate your insight!

// in CI (GitHub Actions), it will pull the `azurite` image as a Windows container and Microsoft
// doesn't ship Windows containers of the `azurite` image, so we just ignore the test alltogether
//
// TODO(@auguwu): how to fix this? :3
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be possible to split this out into a separate issue?

stream.write_all(&options.data[..]).await?;
stream.close().await

// TODO(@auguwu): add metadata to document that was created and the given content type
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, can this be split out into its own issue?

@auguwu
Copy link
Member Author

auguwu commented Jan 15, 2024

I left only quick comments asking to please make TODOs in source also become GitHub issues - would appreciate your insight!

I'll make marked TODOs as issues once this PR is merged.

The library I wanted to use is not what I had planned in mind of designing the `remi-gcs` crate. There can be a
community maintained one or when people request Google Cloud Storage support, then we'll talk about it via issues.
@auguwu auguwu changed the title Redesign crates, add Azure and GCS support Redesign crates, add Azure support Jan 15, 2024
@auguwu
Copy link
Member Author

auguwu commented Jan 15, 2024

I decided to not work on remi-gcs, as I'll plan to do it soon when there is an officially maintained Google Cloud SDK that is reliable or a community-maintained version can be present if wished.

@auguwu auguwu merged commit 1689362 into master Jan 15, 2024
7 checks passed
@auguwu auguwu deleted the feat/crate-redesign branch January 15, 2024 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants