Skip to content

Conversation

@lmtroper
Copy link
Contributor

@lmtroper lmtroper commented Mar 4, 2024

Changelogs

Integrated a basic write implementation for Zarr files with the Polaris FSSpec. Some of the major changes include:

  • Added a new endpoint in the client to be able to write a zarr file of a Polaris dataset.
  • Added two new methods in the FSSpec: rm and pipe_file

More details of the FSSpec Implementation:

  • The rm method is currently added as placeholder since it's required for the Zarr writing integration.
  • The pipe_file method calls the hub to get a signed URL for a PUT request, then writes the data to the bucket. The S3 bucket uses SHA-256 for data security but it is currently set to None.

Checklist:

  • Was this PR discussed in an issue? It is recommended to first discuss a new feature into a GitHub issue before opening a PR.
  • Update the API documentation if a new function is added, or an existing one is deleted.
  • Write concise and explanatory changelogs above.
  • If possible, assign one of the following labels to the PR: feature, fix or test (or ask a maintainer to do it for you).

@lmtroper lmtroper requested a review from cwognum March 4, 2024 17:19
Copy link
Collaborator

@cwognum cwognum left a comment

Choose a reason for hiding this comment

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

This look great already! Thank you for taking the time to document everything! 🙏

For the failing code checks, it seems there is a mismatch between the line length we expect and the line length used. This should be automatic, as it is configured in the pyproject.toml. After navigating to the root directory, you should be able to just run:

ruff format .

It is a little weird though, because the error message states:

Would reformat: polaris/hub/client.py
Would reformat: polaris/utils/types.py
2 files would be reformatted, 43 files already formatted
Error: Process completed with exit code 1.

But types.py is fine for me locally. Let's try again and I'll look into it some more if it's not fixed.

@cwognum cwognum added the feature Annotates any PR that adds new features; Used in the release process label Mar 4, 2024
@lmtroper lmtroper requested review from cwognum and removed request for cwognum March 6, 2024 15:52
@lmtroper lmtroper requested a review from cwognum March 6, 2024 19:56
Copy link
Collaborator

@cwognum cwognum left a comment

Choose a reason for hiding this comment

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

Thanks @lmtroper ! Almost there!

@lmtroper lmtroper requested a review from cwognum March 7, 2024 17:35
Copy link
Collaborator

@cwognum cwognum left a comment

Choose a reason for hiding this comment

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

Very polished! 💅

One minor comment still, but this is great work! Thank you @lmtroper !

@cwognum
Copy link
Collaborator

cwognum commented Mar 7, 2024

Feel free to merge this PR whenever you're ready! 🙏

@lmtroper lmtroper merged commit 4e38be3 into main Mar 8, 2024
@lmtroper lmtroper deleted the feat-polaris-fsspec-write-implementation branch March 8, 2024 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature Annotates any PR that adds new features; Used in the release process

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants