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

WIP: adding azure blob as object store #946

Closed

Conversation

piyushsingariya
Copy link

Fixes #687 .

Description

This PR aims to add support for Azure Blob Storage as Object Store in parseable.

The Solution/Changes implemented in this PR are as follows:

  • Add a separate CLI subcommand similar to s3-store
  • Add Config/Vars required to connect to Azure Blob Storage
  • Alter + Rename previously written S3 struct into a generic struct to use LimitStore better and to enable re-usability of code for Azure Blob as well.

Note

I've only started learning Rust less than a week ago, so I am unfamiliar with Rust ecosystem. Any guidance or feedback would be greatly appreciated as I work on this task.


This PR has:

  • been tested to ensure log ingestion and log query works.
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added documentation for new or modified features or behaviors.

Copy link
Contributor

github-actions bot commented Sep 29, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@piyushsingariya
Copy link
Author

I have read the CLA Document and I hereby sign the CLA

nitisht added a commit to parseablehq/.github that referenced this pull request Sep 29, 2024
@nikhilsinhaparseable
Copy link
Contributor

@piyushsingariya for this to work, you need to implement all the methods defined under trait ObjectStorage
currently you will find two implementations for this trait (one for s3-store, other for local-store) -

  1. https://github.com/parseablehq/parseable/blob/main/server/src/storage/s3.rs
  2. https://github.com/parseablehq/parseable/blob/main/server/src/storage/localfs.rs

@piyushsingariya
Copy link
Author

piyushsingariya commented Oct 5, 2024

Hi @nikhilsinhaparseable,

I've reutilized the pre-written implementation of S3 for trait ObjectStorage, since S3 is initializing a client with LimitStore<AmazonS3> as LimitStore<ObjectStore>, I've avoided re-implementing and reuse the same code for AzureBlob via initialzing an instance of LimitStore<MicrosoftAzure> as LimitStore<ObjectStore>.

I've achieved this via altering the previous S3 struct to be generic and to accept anything that implements the trait ObjectStore from object_store crate.

Signed-off-by: Piyush Singariya <piyushsingariya@gmail.com>
@piyushsingariya
Copy link
Author

@nikhilsinhaparseable It would be easier if you could checkout my pr in local with gh cli, that way it would be easier in understanding how we don't need to rewrite the implementation.

Do let me know what else needs to be fixed here, since I am really looking forward to getting this PR merged.

@nitisht
Copy link
Member

nitisht commented Oct 5, 2024

@piyushsingariya we had a customer waiting for this so @nikhilsinhaparseable had to work on this here #955 . This effort looks like will take more time. I'd request you to close this PR.

@piyushsingariya
Copy link
Author

I'm quite disheartened by how this has turned out. I spent a few days working on my PR, hoping to contribute meaningfully, it sat unreviewed for a week.

Though I understand the priority shift and urgency but this feels highly discouraging and doesn't align with the open-source spirit of collaboration. I truly hope this isn't indicative of how external contributions are generally handled.

I hope this feedback helps improve the process for other contributors. Closing this PR.

@nitisht
Copy link
Member

nitisht commented Oct 5, 2024

Thanks @piyushsingariya

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.

add azure blobstore support as object store backend
3 participants