Skip to content
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

Add /setup endpoint #223

Merged
merged 17 commits into from
Dec 7, 2021
Merged

Add /setup endpoint #223

merged 17 commits into from
Dec 7, 2021

Conversation

iMicknl
Copy link
Owner

@iMicknl iMicknl commented Oct 29, 2021

  • Implement better caching (for get_devices and get_gateways)
  • Add tests (with multiple JSON files)

@github-actions github-actions bot added the enhancement New feature or request label Oct 29, 2021
@iMicknl iMicknl marked this pull request as draft October 29, 2021 21:37
@iMicknl iMicknl marked this pull request as ready for review October 31, 2021 17:44
@iMicknl iMicknl linked an issue Oct 31, 2021 that may be closed by this pull request
@iMicknl
Copy link
Owner Author

iMicknl commented Nov 1, 2021

I would love if we can just do:

(devices, places, gateways) = await client.get_setup()

However, this is currently not supported:

TypeError: cannot unpack non-iterable Setup object.

Any ideas?

.pylintrc Show resolved Hide resolved
@iMicknl iMicknl mentioned this pull request Nov 1, 2021
@iMicknl iMicknl requested a review from tetienne November 3, 2021 18:15
pyhoma/models.py Outdated Show resolved Hide resolved
self.devices = devices
if self.setup:
self.setup.devices = devices
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically here, you don’t cache the devices, but sync with the /setup entry point?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? What we do here is overwrite the /setup response with the latest devices response.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK it’s what I can sync :D

By the way, do we use this feature (cache) somewhere?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tetienne not really I think, thus I was also thinking to remove it... However, it doesn't have a lot of extra code and can help other people to make sure they don't call expensive endpoints all the time. :-)

"location": {
"creationTime": 1613674982000,
"lastUpdateTime": 1632527573000,
"city": "*",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be great to have a "real" value to know what the data look like. Same remarks for all the * values.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it really matter? All * values are string values. I could replace the sensitive values with a placeholder, but since * is a string as well, I didn't bother.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum OK.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you like obfuscating more? So we keep the same amount of characters? I have the originals locally, so I can make changes to the regex. Currently I use the same regex as we use (in this PR) to obfuscate sensitive info in the logging.

@tetienne tetienne force-pushed the enhancement/add_setup_endpoint branch from 70acd61 to 283704c Compare December 7, 2021 12:56
@iMicknl iMicknl merged commit 82f0cc8 into master Dec 7, 2021
@iMicknl iMicknl deleted the enhancement/add_setup_endpoint branch December 7, 2021 13:39
tetienne added a commit that referenced this pull request Feb 14, 2023
* Add /setup endpoint

* Update location class

* Fix obfuscate id

* Obfuscate id

* Add mask feature

* Add basic fixtures for /setup

* Add basic tests

* bugfix

* Add local setup json

* Add feature key

* Update fixtures

* Add better api doc

* Type zone object

* Add extra fixture

* Update pyhoma/models.py

Co-authored-by: Thibaut <thibaut@etienne.pw>

* Refactor based on feedback

* Fix style

Co-authored-by: Thibaut <thibaut@etienne.pw>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add /setup endpoint
2 participants