Mbeliaev/update props

[beliaev-maksim]

fix #65

[allburov]

I agree with #65 and like the solution!

But I don't like the effort and code amount that is required to fix #293

For instance, I see the next problem - when someone will update properties like these:

path.properties['my-prop-1'] = 'test'
path.properties['my-prop-2'] = 'test2'
path.properties['my-prop-and-more'] = 'test-value'

It'll generate 3 requests in the row (or even 6 times, because path.properties will hit API every time

We can instead return "ReadOnlyDict" instance when one gets path.properties and then write a warning about "How to use properties in the library right" if one wants to set value.

For instance, django does not hit DB every times when you write a property and waits until obj.save() method is called. We can do similar way - obj.save(properties=True), but we don't have right architecture for this yet. Let's postpone the fix for the issue #293 a little

[beliaev-maksim]

@allburov
what about to keep the implementation, just document parts that are recommended (eg single request). Then it is up to the user to decide what to use for them to get higher efficiency

if you have
props = {...}
we will recommend:
props.update({...})

this calls single request

[beliaev-maksim]

+@OddBloke as a requestor

I think it is reasonable trade-off for now to send request on each update. We can well document it and explain that dict.update() will be better for production
thus, I do not see any significant reason why not to merge this change

[allburov]

what about to keep the implementation, just document parts that are recommended (eg single request)

Usually people look at documentation when they already have about 10 requests for 1 artifact and have 10 artifacts that they are changing (10*10=100 requests).

The suggested implementation changes the current behavior - if one use it as "set per-property" it'll generate a lot of unnecessary requests.

With dict.update() what the difference between

props = artifact.properties
props['my-prop'] = 'value'
artifact.properties = props

and

props = artifact.properties
props['my-prop'] = 'value'
props.update()

then?

3 lines, the same way "one has to known something about the library"

My suggestion is - let's keep "update_properties" in one request and merge it.

I'm not sure that we should fix #293 at all, it doesn't add a new functional, just a few lines shorter solution, but it WILL add more code in the library. But let's talk about it there #293, if you want to

[OddBloke]

Hey, thanks for this change! I've tested, and it does address my specific use case. However, I also agree that this could be a surprising (and costly) change for existing consumers of the API. (I've also commented on #293, so we don't have the conversation happening in two places.)

[beliaev-maksim]

@allburov
what is with CI?
status never updated