fix incorrect response on properties

[xiekeyang]

it return a successful result for GET/PUT properties, even repository is invalid. Therefore, it should add the judgement before the operation. Thanks.

[xiekeyang]

@dmp42 Thanks a lot.

[dmp42]

@shin- ping

[shin-]

That's not necessary, because that case will never happen.

[xiekeyang]

@shin- really? However, when I use GET http://myregistry.io/v1/repositories/namespace/invalid_repo/properties it returns {"access": "private"}, which is incorrect. for users who build their own index, this kind of rest API is necessary, and the bug will happen possibly. As other APIs code in registry, it seems had better to add the prejudging. Thanks

[shin-]

Your index implementation should be aware of repositories existence and simply not make that call if the repository doesn't exist.

This PR as it stands will just degrade the official registry performance, which I'm not okay with.

[xiekeyang]

Yes. So if the condition in set_properties like if not data or not isinstance(data, dict): return toolkit.api_error('Invalid data') will also degrade the registry performance? Definitely, the index can and should be aware of data valid. If i should remove this line? Thanks

[shin-]

You do realize there's a difference in time cost between checking the type of a variable and accessing a network filesystem, right? I don't think your argument is very solid there.

[xiekeyang]

Right, but I'm just some confused. I agree it is important of high performance. But firstly, The registry likely have to avoid illogical circle inside it, right? Repository is saved in registry, but not in index. I agree that it can be avoided by checking Repository firstly. But when I use this API directly, and get incorrect response, it make me feel this function not good. There are similar weak points in the registry, like invalid properties data setting making registry return true, invalid image id being accepted when building a new repo, etc. I know these can be avoid by pre-checking, but its circle is more likely illogical. Now I'm not sure if you regard them as bug, and if I should make analysis. what do you think? Thanks.

[shin-]

Repository is saved in registry, but not in index.

The index's primary purpose is to have a record of the repositories that were pushed to your registry (as the name suggests). It should definitely be aware of their existence.

But when I use this API directly, and get incorrect response, it make me feel this function not good.

It is not meant to be used directly, at least in the official implementation. As an implementer, it is your responsibility to conform to this expectation, not the other way around.

Basically:

1. This function is only used in the context of non-standalone registries (i.e. coupled with an index). There are no private repositories in standalone registries.
2. The most important implementation of the non-standalone is the official one. It serves thousands of users and can not afford to needlessly sacrifice performance.
3. People are welcome to create other alternative index implementations, and we will do our best to accomodate this, but (2) will always take precendence in cases where there is a trade-off.
4. This is an open-source project, if you feel that it doesn't fit your expectations in terms of API, I certainly encourage you to create a fork that will better suit your needs. I don't mean this in a bad way, either: some of the things you may see as incorrect are conscious choices we made when implementing this, and it's totally fine to have a different opinion about those choices.

I hope this helps explaining my point of view once and for all.

[xiekeyang]

Thanks a lot for your explain!