search

xperimental / netatmo-exporter

Prometheus exporter for Netatmo sensor data.
MIT License
44 stars 18 forks source link

Write token file upon refresh #54

Open RichieB2B opened 11 months ago

RichieB2B commented 11 months ago

I just started using the new OAuth using the toke file. It works great, thanks! I did notice however that the token file is only written when netatmo-explorer exists. I think it would be better to save the new tokens every time they are refreshed. In case of a crash the new tokens are currently lost.

xperimental commented 11 months ago

Currently the token rotation is completely handled by the oauth library I use, so the "netatmo-exporter code" does not know when a refresh actually happens, it is completely transparent to it. It just reads the (possibly changed) token from the library on exit.

Saving the token on change / periodically is something I had thought about before as well, but did not get to implementing so far.

RichieB2B commented 11 months ago

Perhaps you can check if the expiry has changed and only save the token file when it has. Once every few minutes should be enough.

xperimental commented 11 months ago

@RichieB2B Just realized something: I think the more important question for me is: why did it crash? Do you have any logs of the crash that we can use to fix that issue?

RichieB2B commented 11 months ago

netatmo-exporter doesn't crash often, the last time was 6 months and many versions ago. Not worth investigating now. It's just that you can't count on a clean process exit 100% of the time.

xperimental commented 11 months ago

Ok. I thought this occurred to you because of a recent crash and was worried there might be a bigger issue here than just "does not save token". :relieved:

I do know that there's no "100% stable" but I surely try to make it as reliable as possible :slightly_smiling_face:

xperimental commented 2 weeks ago

It looks to me as if more people are experiencing errors writing the token file on shutdown (see mentioned issue).

I have come around to implementing a "save on token change" mechanism, but I am still looking at how to integrate this into the existing code. The token handling is done in the oauth2 library and not my own code, so I'll have to move around a few bits first.

szermatt commented 1 week ago

I have the same problem, usually caused by power outages, which are unfortunately very common.

I fixed it for now with the following (draft) changes: https://github.com/xperimental/netatmo-api-go/compare/master...szermatt:netatmo-api-go:savetoken https://github.com/xperimental/netatmo-exporter/compare/master...szermatt:netatmo-exporter:savetoken

It modifies your copy of netatmo-api-go; I couldn't see an alternative. I do wonder whether it wouldn't be more straightforward to just do the oauth2 stuff inside netatmo-exporter.

I was getting ready to send them out, but you're already working on it, so I just thought I'd send them out FYI. Feel free to disregard them completely, to copy from them, or, if you prefer, let me know if I should clean them up and sent out pull request for a real review.

Thanks!

xperimental commented 1 week ago

@szermatt Thanks for the contribution. I'm currently on vacation, but I'll have a look once I'm back home.