search

devopshq / artifactory

dohq-artifactory: a Python client for Artifactory
https://devopshq.github.io/artifactory/
MIT License
273 stars 144 forks source link

`find_repository` silently fails without a Pro license #356

Closed briantist closed 2 years ago

briantist commented 2 years ago

The calls to find_repository (and the specific versions) seem to expect that a 400 error means the repository can't be found, and so it doesn't raise an error and returns as if there's no result:

https://github.com/devopshq/artifactory/blob/2fa2b64a9c5b84f595ca9145f7fb07369608b91a/dohq_artifactory/admin.py#L140-L144

But the OSS version of Artifactory hides a bunch of repository API calls behind a paywall, and responds with 400:

This REST API is available only in Artifactory Pro (see: jfrog.com/artifactory/features). If you are already running Artifactory Pro please make sure your server is activated with a valid license key.

In this case, these methods act like the repository doesn't exist even though it does, and other calls that don't use those APIs will still work.

For example I've been able to work around this with something like this:

    for repo in path.get_repositories():
        if repo.name == name:
           return repo

I didn't dig deep enough to see what API call(s) get_repositories() uses but it seems to work fine on the OSS version.

I don't quite think that the correct course of action is to have find_repository() fall back to that method necessarily, but I do think it should try to distinguish between this one particular type of 400 response (I guess by message matching) and raise the exception, to make it more obvious what's going on. It could optionally offer get_repositories() as an alternative, or link to documentation on it, or something, but that's not strictly necessary.

briantist commented 2 years ago

@fuzzmz @allburov thanks for that improvement in #383 . Could you clarify what that means for using find_repository()?

I may be reading the code wrong but it seems like the problem I describe here will still happen, and that the only way to know it will be if you're looking at debug output.

The issue is that on a pro license 400, the routine will still return as though the repository doesn't exist.

Could it be that in the branch for the 400 status that changes the debug message, it should simply not return False, and then pass through to where raise_for_status() would handle it?