Skip to content
This repository was archived by the owner on Sep 12, 2018. It is now read-only.

Add refcount extension#606

Closed
ncdc wants to merge 1 commit intodocker-archive:masterfrom
ncdc:refcount-extension
Closed

Add refcount extension#606
ncdc wants to merge 1 commit intodocker-archive:masterfrom
ncdc:refcount-extension

Conversation

@ncdc
Copy link
Copy Markdown
Contributor

@ncdc ncdc commented Oct 3, 2014

Add refcount extension to store image references and cleanup orphans.
Based heavily on @wking's work in
#409 - this is just an
adaptation to extension format.

Signed-off-by: Andy Goldstein agoldste@redhat.com

@ncdc ncdc force-pushed the refcount-extension branch 2 times, most recently from e6967e5 to c7496dc Compare October 3, 2014 20:13
@ncdc
Copy link
Copy Markdown
Contributor Author

ncdc commented Oct 3, 2014

@wking @dmp42 thoughts? There are some TODOs in the extension's setup.py so I'd appreciate your input on those as well as the overall format and structure of the extension.

@ncdc
Copy link
Copy Markdown
Contributor Author

ncdc commented Oct 3, 2014

Assuming you like this, how do we get flake8 to ignore the setup.py in the extension's folder, or do I just need to fix the file to pass the style tests?

@wking
Copy link
Copy Markdown
Contributor

wking commented Oct 3, 2014

I haven't actually gone over the implementation yet, but I'd drop the
“[refcount]” prefix from the logger calls. If we want to prefix
logged information with module names (and I think we do) we should add
“%(name)s” to the logging.Formatter in docker_registry.wsgi. But that
would be a separate PR.

@dmp42
Copy link
Copy Markdown
Contributor

dmp42 commented Oct 5, 2014

@ncdc re: flake, tweak this: https://github.com/docker/docker-registry/blob/master/tox.ini#L6

@ncdc ncdc force-pushed the refcount-extension branch 2 times, most recently from 1211a16 to 752e973 Compare October 6, 2014 13:44
@ncdc
Copy link
Copy Markdown
Contributor Author

ncdc commented Oct 6, 2014

@wking I created a separate PR for the log format: #608

@dmp42
Copy link
Copy Markdown
Contributor

dmp42 commented Oct 6, 2014

Guys, like I said, I'm happy with the community owning extensions. So give me a ping when:

  • things are complete
  • tests are happy
  • you gathered at least one LGTM review

and I'll merge (eg: don't wait for a review from me on these).

dmp42
dmp42 reviewed Oct 6, 2014
View reviewed changes
extensions/refcount/setup.py Outdated
Copy link
Copy Markdown
Contributor

@dmp42 dmp42 Oct 6, 2014

Choose a reason for hiding this comment

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

I would use a separate README and LICENSE for the extension.

@wking wking mentioned this pull request Oct 9, 2014
Closed
Add refcount extension
4e5f2e9 
Add refcount extension to store image references and cleanup orphans.
Based heavily on @wking's work in
docker-archive#409 - this is just an
adaptation to extension format.

Signed-off-by: Andy Goldstein <agoldste@redhat.com>
@ncdc ncdc force-pushed the refcount-extension branch from 752e973 to 4e5f2e9 Compare October 13, 2014 16:21
@ncdc
Copy link
Copy Markdown
Contributor Author

ncdc commented Oct 13, 2014

@wking I added a license and readme for the extension. Also, I changed the author in setup.py to you, which I think makes sense if you're ok with it. I still have TODO for author_email and url...

@ncdc
Copy link
Copy Markdown
Contributor Author

ncdc commented Oct 13, 2014

1 other thing - how would you write tests for this?

@wking wking mentioned this pull request Oct 22, 2014
Open
@wking wking mentioned this pull request Nov 10, 2014
Open
@dmp42
Copy link
Copy Markdown
Contributor

dmp42 commented Dec 2, 2014

@wking @ncdc where do we stand with this?

@wking
Copy link
Copy Markdown
Contributor

wking commented Dec 2, 2014

On Tue, Dec 02, 2014 at 02:30:34PM -0800, Olivier Gambier wrote:

@wking @ncdc where do we stand with this?

Do we care about supporting this for the Python registry? Folks have
survived so far without it built in, and while I think we're still
providing maintenance for the Python implementation, I don't see the
point in investing time in adding new features there.

@matthughes
Copy link
Copy Markdown

matthughes commented Dec 3, 2014

@wking Is there a timeline for the go rewrite? I had not even heard about it until now. Just curious.

@wking
Copy link
Copy Markdown
Contributor

wking commented Dec 3, 2014

On Tue, Dec 02, 2014 at 05:19:54PM -0800, Matt Hughes wrote:

Is there a timeline for the go rewrite?

Alpha is due 2014-11-24, and the first RC is due 2015-01-12 1.

@dmp42
Copy link
Copy Markdown
Contributor

dmp42 commented Dec 4, 2014

@wking I'm happy with it either way.

@ncdc
Copy link
Copy Markdown
Contributor Author

ncdc commented Dec 4, 2014

@dmp42 we're ok closing this as long as the v2 registry supports pruning/gc in some fashion (either directly or via an extension)

@dmp42
Copy link
Copy Markdown
Contributor

dmp42 commented Dec 4, 2014

I believe v2 will support pruning / gc, either in core or an extension.
Now, mind the roadmap (to be updated soon by @stevvooe) - I don't expect this specific feature to land very soon (more likely sometimes down during Q1) - unless someone starts working on it sooner?

@dmp42 dmp42 closed this Dec 4, 2014
@gesellix gesellix mentioned this pull request Jan 26, 2015
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@ncdc @wking @dmp42 @matthughes