staabm/phpstan-todo-by

http caching

Closed this issue · 4 comments

we should have a http cache builtin, to reduce the impact of http requests on the phpstan analysis process.
I think we just need to cache a url to ticket status mapping.

this will also make sure we won't hammer the APIs of the issue trackers from all todo-by users.

  • as soon as a new comment key is discovered the whole cache should be expired. that way people trying phpstan-todo-by for the first time get nearly non-cached responses all the time
  • the cache should have a short default expiration, maybe 1 hour?
  • the cache life time should be configurable via .neon
  • it should be possible to disable the cache via .neon config
  • the cache should be stored in the phpstan temp folder in a single file

@EmilMassey @DannyvdSluijs any ideas/opinions? anyone of you guys willing to implement the caching?

To be honest I'm very happy with the way it's cached currently by the phpstan itself. For me it's efficient enough and in my case API calls are made rarely thanks to the result cache.

How would this integrate with built-in phpstan cache? Would phpstan clear-result-cache invalidate tickets cache or it would be kept even after running this command?

Personally in my projects I'd probably set this parameter to disable cache, because I want to receive the feedback immediately. Hypothetically I might want to close/resolve the ticket and then run the phpstan (with result cache cleared) right away to make sure that all comments are resolved. If the status was cached at this moment, these comments wouldn't be reported and I would have false belief that there are no unresolved todos.

I don't know how it's going to work but I think cache would be nice feature to have if somebody wants/needs to have it enabled. These APIs when authenticated have quite big rate-limits, but indeed if the codebase has huge amount of todos and phpstan's isn't able to cache effectively, they could hit the limit. But I'd say this is quite rare scenario, so personally I'd prefer no cache as the default.

From the performance point of view, IMO #76 is more interesting but I don't know how to implement it.

From the performance point of view, IMO #76 is more interesting but I don't know how to implement it.

I agree, that this is the more interessting thing. I guess its pretty easy.. maybe I can implement it in the train on the weekend

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.