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

rsc: Allow file blobs in DbOnly Store #1634

Merged
merged 1 commit into from
Aug 22, 2024
Merged

rsc: Allow file blobs in DbOnly Store #1634

merged 1 commit into from
Aug 22, 2024

Conversation

V-FEXrt
Copy link
Contributor

@V-FEXrt V-FEXrt commented Aug 21, 2024

Allows generic blobs to also be created as DbOnly blobs, including the deduping feature.

This change does require that the blob be statd but that should essentially be free because stat is a target and we have to stat every file before upload anyways

@V-FEXrt V-FEXrt requested a review from colinschmidt August 21, 2024 15:28
Copy link
Contributor

@colinschmidt colinschmidt left a comment

Choose a reason for hiding this comment

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

LGTM. A few small questions but nothing blocking.

@@ -631,6 +632,10 @@ export def rscApiGetFileBlob ((CacheSearchBlob _ uri): CacheSearchBlob) (path: S
Pass path

## --- Helper functions ---

def contentTypeFromSize size =
if size < 95 then Some "blob/small" else None
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we determine ~100 was the right limit still?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, we talked about 256 but I'm kicking that down the road


def contentType = match (unsafe_stat file)
Pass (Stat _ _ size) -> contentTypeFromSize size
Fail _ -> None
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering if there is some benefit to failing earlier here rather than waiting for the uploadBlobRequest to fail if it can't stat the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe, I'm not sure which one happens first but I'll at it to the list of things to give a look

@V-FEXrt V-FEXrt merged commit cb8736c into master Aug 22, 2024
11 checks passed
@V-FEXrt V-FEXrt deleted the rsc-file-small-blob2 branch August 22, 2024 21:28
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.

2 participants