keenlabs / KeenClient-Python

Official Python client for the Keen IO API. Build analytics features directly into your Python apps.
https://keen.io/docs
MIT License
133 stars 58 forks source link

Saved Query partial update #123

Closed masojus closed 7 years ago

masojus commented 7 years ago

This change hasn't been properly tested and would need to be cleaned up before getting merged. The purpose is to show what would need to happen to enable partial updates to a Saved/Cached Query while trying to preserve remote changes (e.g. - changes made via the Project Explorer Web UI), and to discuss that a little bit.

josephwegner commented 7 years ago

Thoughts on changing update to do the partial updates, and add a new func update_full that does the whole thing. As it stands, I think leaving the default update as a full overwrite is still a big surface area for mistakes.

update should probably fall back to update_full if it is not passed the old definition.

Does that make any sense? Good idea? Bad idea?

masojus commented 7 years ago

I think if client devs are already using update() with a full definition, and not editing their queries elsewhere, then it's a good idea to keep supporting that flow, as you describe. I could think of better names if we could make the intent clearer and would accept input there, of course :)

masojus commented 7 years ago

I pushed the readme changes in a separate PR since those need to happen regardless. I'll work on cleaning this up, incorporating PR feedback and getting some tests in place.

masojus commented 7 years ago

I also need to update the README file to match this behavior.

josephwegner commented 7 years ago

This looks pretty good. Can we add some tests that check the different functionality of update and update_full? Or is that functionality untestable?

masojus commented 7 years ago

I can look into testing that giving a full definition behaves differently from a partial update in terms of absent properties.