Skip to content

feat: Public GC function of oci.Store#656

Merged
Wwwsylvia merged 25 commits into
oras-project:mainfrom
wangxiaoxuan273:public-gc
Jan 11, 2024
Merged

feat: Public GC function of oci.Store#656
Wwwsylvia merged 25 commits into
oras-project:mainfrom
wangxiaoxuan273:public-gc

Conversation

@wangxiaoxuan273

@wangxiaoxuan273 wangxiaoxuan273 commented Dec 8, 2023

Copy link
Copy Markdown
Contributor

Part of #472

@wangxiaoxuan273 wangxiaoxuan273 changed the title Public gc feat: Public GC function of oci.Store Dec 11, 2023
@wangxiaoxuan273
wangxiaoxuan273 marked this pull request as ready for review December 11, 2023 07:17
@codecov-commenter

codecov-commenter commented Dec 11, 2023

Copy link
Copy Markdown

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (c34d275) 75.57% compared to head (20af59f) 75.71%.

Files Patch % Lines
content/oci/oci.go 77.61% 10 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #656      +/-   ##
==========================================
+ Coverage   75.57%   75.71%   +0.13%     
==========================================
  Files          59       59              
  Lines        5692     5765      +73     
==========================================
+ Hits         4302     4365      +63     
- Misses       1023     1028       +5     
- Partials      367      372       +5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Comment thread content/oci/oci.go
@wangxiaoxuan273
wangxiaoxuan273 marked this pull request as draft December 18, 2023 06:21
@wangxiaoxuan273
wangxiaoxuan273 marked this pull request as ready for review December 25, 2023 02:15
Comment thread internal/graph/memory.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread internal/graph/memory.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go
Comment thread content/oci/oci.go
Comment thread internal/graph/memory.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go

@eiffel-fl eiffel-fl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi!

This looks really cool!
I tested it and did not find any problem (I am still having blobs not deleted but I am thinking more and more this is because we are doing something wrong on our side).
I just have one comment regarding the TODO but nothing critical.

Best regards.

Comment thread content/oci/oci.go Outdated

@eiffel-fl eiffel-fl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi!

I tested it and it works fine! I just found one small nit but unless I am missing something I think we are near the merge.

Best regards.

Comment thread internal/graph/memory.go Outdated

@eiffel-fl eiffel-fl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi!

I tested it and I like the new approach of using index.json!
I am still getting the same behavior but this is not related to this code (see #664).
I just found one small nit, otherwise, from my side, it is ready to go (note that I am not maintainer here, so just grey checkmark)!

Best regards.

Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go
@wangxiaoxuan273

wangxiaoxuan273 commented Jan 9, 2024

Copy link
Copy Markdown
Contributor Author

Current behavior regarding referrers: when a referrer manifest is not in index.json, it will be cleaned by GC. Change may be made in the future when OCI layout has better support for finding referrers.

Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go
Comment thread content/oci/oci.go Outdated

@shizhMSFT shizhMSFT left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@Wwwsylvia Wwwsylvia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM with a few nit suggestions. Please rebase and resolve comments to unblock merge.

Comment thread content/oci/oci.go Outdated
Comment thread content/oci/oci.go Outdated
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>
Signed-off-by: Xiaoxuan Wang <wangxiaoxuan119@gmail.com>

@Wwwsylvia Wwwsylvia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@Wwwsylvia
Wwwsylvia merged commit 3b1dd0e into oras-project:main Jan 11, 2024
@wangxiaoxuan273
wangxiaoxuan273 deleted the public-gc branch January 11, 2024 06:07
@Wwwsylvia Wwwsylvia mentioned this pull request Jan 26, 2024
4 tasks
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.

6 participants