Skip to content

google cloud storage cachestore #18

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 25 commits into from
Oct 9, 2024
Merged

google cloud storage cachestore #18

merged 25 commits into from
Oct 9, 2024

Conversation

marino39
Copy link
Contributor

@marino39 marino39 commented Oct 7, 2024

No description provided.

@marino39 marino39 marked this pull request as ready for review October 8, 2024 09:27
@marino39 marino39 changed the title MVP: google cloud storage cachestore google cloud storage cachestore Oct 8, 2024
VojtechVitek
VojtechVitek reviewed Oct 8, 2024
View reviewed changes
Copy link
Member

@VojtechVitek VojtechVitek left a comment

Choose a reason for hiding this comment

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

LGTM

So we support Expiry but not Lock methods.

Does the GCP bucket allow us to set any expiry? Or are there any limitations on the duration?

@marino39
Copy link
Contributor Author

marino39 commented Oct 8, 2024

LGTM

So we support Expiry but not Lock methods.

Does the GCP bucket allow us to set any expiry? Or are there any limitations on the duration?

I haven't found anything. I have only found object lifecycle policies but those need to be set on the whole bucket. Maybe David has some idea how to provide better implementation.

david-littlefarmer
david-littlefarmer reviewed Oct 8, 2024
View reviewed changes
Copy link
Member

@david-littlefarmer david-littlefarmer left a comment

Choose a reason for hiding this comment

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

Lot of comments, lot of nits, and some suggestions.
I was also thinking if gcstorage is right name, but is feels that it is right.

Great job with tests!!!! 👍 👍 👍

marino39 and others added 14 commits October 8, 2024 14:16
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
1a2abf7 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
46d347b 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
7347d19 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
b817aab 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
7955b04 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage_test.go
b955fc8 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
f654b48 
Co-authored-by: David Sedláček <[email protected]>
@marino39
refactor + apply review remarks
cdfa671
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
c447ba8 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
e2dcfc9 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
80de574 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
9a161cc 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
e9d30aa 
Co-authored-by: David Sedláček <[email protected]>
@marino39
apply review remarks
4323bad
david-littlefarmer
david-littlefarmer reviewed Oct 8, 2024
View reviewed changes
Copy link
Member

@david-littlefarmer david-littlefarmer left a comment

Choose a reason for hiding this comment

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

2 nits, one question

@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
37bb7fe 
Co-authored-by: David Sedláček <[email protected]>
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
98aa168 
Co-authored-by: David Sedláček <[email protected]>
david-littlefarmer
david-littlefarmer reviewed Oct 8, 2024
View reviewed changes
@marino39 @david-littlefarmer
Update gcstorage/gcstorage.go
c4deac1 
Co-authored-by: David Sedláček <[email protected]>
VojtechVitek
VojtechVitek approved these changes Oct 9, 2024
View reviewed changes
Copy link
Member

@VojtechVitek VojtechVitek left a comment

Choose a reason for hiding this comment

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

LGTM

@marino39 marino39 merged commit 75090a8 into master Oct 9, 2024
@marino39 marino39 deleted the gcstorage branch October 9, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@david-littlefarmer david-littlefarmer david-littlefarmer left review comments

@VojtechVitek VojtechVitek VojtechVitek approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@marino39 @VojtechVitek @david-littlefarmer