-
Notifications
You must be signed in to change notification settings - Fork 57
added support for using the api for all jfrog api's #224
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Hey @Ido-Zerah , |
|
hi thank you for the reply, |
|
additionally regarding the tests failing this appears to be because of codacy and not the code actually failing tests. |
JFrog has actually moved the Access API from @anancarv: Regarding the version compatibility, our choices seem to be:
I think I'd prefer the former just to keep things simple. |
Ahh ok, I didn't notice that change in the Access API. I also prefer the first option as it is easier. But, sooner or later, we would need to tackle this issue because we cannot prevent people from upgrading their Artifactory version. So let's fix it. I would suggest doing:
What do you think? |
| ) # to support base urls with or without /artifactory suffix | ||
| response: Response = http_method( | ||
| f"{self._artifactory.url}/{route}", | ||
| f"{uri}/{art_service}/{route}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't yet fix the issue. Actually with the current commit if I create an Artifactory(url='http://localhost:6543/' and check the behaviour, now create_access_token requests are also directed to the path /artifactory/access/api. It would be better to have a commit or commits that direct access requests to the correct path.
In principle this would be the least disruptive way of adding client transparent support for older versions. In the code, |
Yes you're right. Let's not implement transparent report yet then, and update the README as you suggested. |
Description
as of now the artifactory api included different routes, som atart with artifactory/api/... however some of the api routes use the other services other that the api.
example:
access, xray, topology and such.
in order to add features that use the other routes apis i added the art-service parameter. this will appended between the base url and the route variable.
in addition in order for the pre-commit-hooks to work i needed to add a dependency to the bandit precommit
Type of change
Please delete options that are not relevant.
How has it been tested ?
Checklist: