Extended Registry API v2 Support (images details, labels support, tag list) #84
Conversation
|
Hi @arthurdk . Thank you for this big addition to the project and your kind words. I really appreciate it. Let me have a look at the changes. |
| /** | ||
| * Calculates the total download size for the image based on | ||
| * it's ancestry. | ||
| */ | ||
| /* |
There was a problem hiding this comment.
If you effectively delete code by commenting it out, please simply remove it instead of keeping it. I left it there to pick it up when I have time. Now that you've implemented it, there's now need to keep the old code around unless there's something you want to implement in the future.
There was a problem hiding this comment.
That(s because I already started to implement it but anyway there are a lot of comments to remove I agree.
|
So I've made the changes but I kept the comment on the size divs as I'm working on it. (see If you want to, you can wait to merge the PR until I finish the work on the image size. |
|
I will do another PR for the size feature, as I don't have enough time to finish it until some days. So if you don't find problems then this batch is ready to go. :) |
| @@ -20,11 +20,18 @@ angular.module('tag-controller', ['registry-services']) | |||
| $scope.repositoryName = $route.current.params.repositoryName; | |||
| $scope.repository = $scope.repositoryUser + '/' + $scope.repositoryName; | |||
| $scope.tagName = $route.current.params.tagName; | |||
|
|
|||
| // How to query the tags | |||
| $scope.tagsDetails = []; | |||
There was a problem hiding this comment.
This tagsDetails looked suspicious to me since you fill the tags with all details below. By grepping through the code of yours I found that is not used any longer. There fore I suggest to remove this line.
|
Your latest suggestions are implemented. If |
|
Any news? Really looking for merge! :) |
|
@arthurdk Thank you for all your work. I would really like to merge this PR but what's hindering me from doing so is the amount of manifest Let's brainstorm what our options are, shall we? I don't see a way to get rid of the manifest |
|
@kwk Yeah I commented out the code that calculate image/layer size as I still have problem to get it totally working. (it'll be implemented for later) I agree with the facts that it generate a lot of requests. If the last registry version is still ignoring with the parameters for pagination (I'll check this too) we can use a temporary solution: only display tag name. I agree that its not optimal but right now it'll be better than nothing. |
|
Ok, so I managed to get the registry working with pagination (I'm talking about the tag So in the mean time we appear to have 2 choices :
|
|
@arthurdk Can you please share the code changes you did by checking them into a branch of your fork of docker/distribution? It doesn't matter if it is not polished. You can always Let's recap, we both think that pagination is one solution to the problem of too many requests per page of a tag listing, right? Although I would normally vote against a simulation of pagination there are some benefits to it. Pros of simulation:
Cons of simulation:
I have to think about this. But I'm in favor of not building something that is broken by design. |
|
@kwk It's planned, I'll share the code on my fork as soon as possible. I managed to get an almost working implementation but it lacked the link header which permits correct pagination (see. the pagination documentation). I do not have this version any more (I didn't commit this one as it was just a "proof of concept") as I'm working on the proper implementation. So I think I'll have something to share this weekend. I totally agree with your recap and simulation seems to be the right choice from an user point of view. But regarding the way repository pagination is implemented (i.e. it can list repositories up to a 100 at maximum) so if the same happen for tag pagination in the future, it'll break our simulation. Simulation of pagination requires to have the ability to fetch all tags at once and its something that might change. In my opinion its something we also have to discuss on Even if this happen we'll still be able to fetch all tags with n requests (n depending on the maximum numbers of tags we can get in a response and the number of tags stored in the registry). But it will still be risky to send a lot a requests because if some of them fail, our pagination system might broke. So simulate the pagination will be doable but kind of risky and as you listed them in your pros it can bring features such as previous page etc. I don't know if I'm thinking too far ahead of this problem or not. What do you think of it? |
|
@kwk Sorry for the delay, I was a little busy. I added a basic pagination system for tags and set the default number to 10 (see readme). Configuration is also possible via the corresponding environment variable. |
|
Hi @arthurdk Wow, I wasn't aware you would actually go about and implement this. I try to check it out tomorrow. |
|
I thought that having the possibility to fetch images details is a nice feature so I'm doing everything I can so its fully working. :) But my solution is basic and as I was saying the API implementation might change. In the mean time its simple and working so in my opinion it's good to have it even if it may be changed in the future. (I'll look into that as I am following registry development). |
|
Hi @kwk , is the removal of the label an error or did you find any problems during your review ? |
|
Screenshot has machine information best not tossed out. Steps: I've mitigated it by "touch index.html" in each of the directories in the Tom On Thu, Mar 10, 2016 at 10:19 AM, Arthur De Kimpe notifications@github.com
|
|
@tomsherrod My bad, I was using the dev environment which uses grunt. It is not really related to this PR though as the bug was already present. Anyway it's corrected, the apache config lacked @kwk I just figured out that the Dockerfile in /develop is based on ubuntu whereas the standart one use for release is based on debian jessie. Is it a mistake ? It would be better to have both based on the same parent image. |
|
@arthurdk An observation: Image id shown in the Image details ui is not matching up with the later docker clients. Is this because the image id is being pull from the v1Compatibility data? Am I missing something? |
|
@tomsherrod I think the issue you are having is a bit similar to this one (see also this one). I've experienced the same thing when pulling from Docker Hub with two different host running different version of Docker: when performing |
|
@arthurdk I can confirm that the tags pagination issue is gone. |
|
@kwk It's done, the drop down list is now present and the previous button is replaced by a go to "first page" button. |
|
Hey devs, |
|
Hey @arthurdk . I've manually tested your changes and would like to ask you to squash all your changes into a single commit and force push it to your v2 branch. I'd like to have the chance to revert a PR if something breaks for others and reverting one commit is far more easy than reverting 15 commits. Please make sure your changes are rebased onto the current master. After that I think we can merge it. |
|
github has now the possibility to squash when you merge it. You need to enable this in the repository settings. https://github.com/blog/2141-squash-your-commits |
Revert changes on default projet file Add detailed information for tag listing Removed useless comments, divs, etc -> refactoring Prevent bugs from future changes of API Add digest attribute to Manifest query response Add basic pagination support to tag listing Add environment variable for tags per page Fix bug of image history without config + disabled parent id Fix tags pagination system - Fetch infos for all pages add missing comma Fetch infos for all pages -> simpler fix Disable apache directory listing feature Update tag pagination system to make it feel more like repository pagination system
|
Hi @kwk ! |
|
3 months for 1 pull request?!? -.- |
|
Whouhou !!! 🍻 |
|
🍻🍻🍻 |
|
We all would love to get the docker image updated now 👍 |
|
It seems like the detailed views, etc are disabled because Although it would be nice to also show the details in browse mode... |
Works for me. Or do you mean it's not visible when you set |
|
@FrontSide are you using the develop mode? |
|
Browse only mode shouldn't influence the details. It just disables showing buttons for tag creation an deletion. |
|
@flixr
You need to pull the :v2 image. |
|
I'm also not seeing the details. I have pulled the latest image as such: sudo docker run -d -e ENV_DOCKER_REGISTRY_HOST=myhostfoo.com -e ENV_DOCKER_REGISTRY_PORT=443 -e ENV_DOCKER_REGISTRY_USE_SSL=1 -p 8080:80 konradkleine/docker-registry-frontend:v2 $ docker ps |
That would be good, but in any case I don't see any detailed image info... and yes, I'm using the :v2 image |
|
a note: I notice that the red "Select Repositories to Delete" button flashes briefly when opening the Repositories page, as if they are loading but are suppressed. |
|
Same here, although that shouldn't be the problem.. Screenshot of the latest image pushed to our own registry, browsing with the latest v2 image from docker hub: Any hints on howto debug this? |
|
I just tried the last v2 image pushed to the Docker Hub and it worked just fine on my hand. The command I ran in order to test the detailed view : Please be sure to use the last v2 image available. Are you having errors on your browser console ? @flixr The fact that the git revision is correct but not the view is weird... @kwk Is it possible that older image show the latest git revision ? ( I did not have time to check how the revision is stored/retrieved ) |
|
@arthurdk no that is not possible. But since the version is loaded from a file it might be one of the few things that is not affected when the browser cache hasn't been flushed. Please always try to use a clean browser state when testing. I had trouble seeing changes I made just due to the cache. |
|
Tried with fresh install of FF, details now display. Assuming Chrome what not clearing cache entirely. |
|
Ok, the problem was indeed the cache, specifically In the network monitor I could see that the Last-Modified in the Response header of So is this really expected, or is there something not set up correctly so that it doesn't even attempt to get the latest |
I added some features to this project which work with the current registry API (v2.0):
Re activated the image details view : both image ancestry & size are disabled but the size is kind of easy to implement (i.e. a head request on the blobs end point). Every other things are functional as they were in the v1 branch.
Display Docker image Labels in the image details view
(to do : display labels from the ancestry)
By the way, awesome work with this project and the way you set up the developer environment. Its really easy to use & nice to work with !