devopshq / artifactory

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

0.8.0 sets properties recursively when not desired #326

Closed kvecchione closed 2 years ago

kvecchione commented 2 years ago

To reproduce, use a nested structure in Artifactory and attempt to set a property at a parent level:

e.g. https://artifactory.com/root/L1/L2, want to set on L1 only

path = ArtifactoryPath('https://artifactory.com/root/L1')
properties = path.properties
properties['prop_name'] = 'prop_value'
path.properties = properties

With the new PATCH mechanism, this will set the property on L1 and L2 when the intention was only to set on L1.

Rolling back to 0.7.742 fixes this behavior.

beliaev-maksim commented 2 years ago

looks like a bug by jfrog

in official docs there is a parameter to disable recursive, but it is not considered https://www.jfrog.com/confluence/display/JFROG/Artifactory+REST+API#ArtifactoryRESTAPI-UpdateItemProperties

UPD: I assume user uses older artifactory 6- so, need to add another redundant parameter

thomas-bottesch commented 2 years ago

Just to clarify something up here. We are using artifactory > 7.x and the bug is also occurring there: The correct property that needs to be set is "recursiveProperties". (Which is done by the fix).

It seems that the following documentation is the correct one for newer Artifactory versions: https://www.jfrog.com/confluence/display/RTF6X/Artifactory+REST+API#ArtifactoryRESTAPI-UpdateItemProperties

Dont know why the 'official' one is incorrect.

In my opinion new version of the dohq-artifactory (that integrates this fix) should be pushed to pip since all users of newer Artifactory versions suffer from this bug.

belawaeckerlig commented 2 years ago

I can confirm, that even on version 7.33.12, the Update Item Properties API endpoint is still using 'recursiveProperties' instead of 'recursive'. This seems to be a bug in the documentation. I guess they wanted to change the API, but did not do it to keep it backwards compatible.