search

phochste / RDF-LDF

Perl Linked Data Fragments client
Other
4 stars 4 forks source link

Just remove the caching code #17

Closed kjetilk closed 8 years ago

kjetilk commented 8 years ago

Hi again!

I wonder if this is a better idea... It makes the caching entirely the responsibility of the UA, and uses the existing RDF::Trine mechanism for setting the UA. This passes all my tests.

My only problem is that I don't really have tests to verify that caching actually works... Any suggestions?

What could be helpful is that Test::LWP::UserAgent docs say

One common mechanism to swap out the useragent implementation is via a lazily-built Moose attribute; if no override is provided at construction time, default to LWP::UserAgent->new(%options).

but I have to admit I don't really understand what they mean...

phochste commented 8 years ago

Hi, I'm in the States for code4lib this week and will check this probably when I'm back in Belgium next week

kjetilk commented 8 years ago

OK, cool! :-)

phochste commented 8 years ago

The Test::LWP::UserAgent documentation means that it doesn't implement an own way to override the user_agent. If you need to do that you need to create your own subclass of Test::LWP::UserAgent or provide in LWP::UserAgent::CHICaching a class attribute (e.g. lazy) to set the user_agent at construction time. I don't know if this is possible to have the LWP::UserAgent::CHICaching be a subclass of LWP::Agent or Test::LWP::UserAgent depending on some parameters at construction time.

To get a step forwards the testing of caching should be done in LWP::UserAgent::CHICaching and not in RDF::LDF (as this is now an optional feature).

The question is, should the caching module ( LWP::UserAgent::CHICaching in the example) be specialized for LDF type of queries. Can there be optimised strategies possible when you know you are talking to a LPF endpoint? It certainly makes a lot of sense to put it in a separate module.

But, still I wonder if the caching can't be in some way stay a part of RDF-LDF not only as recommended module. But puggable. Without caching all external modules al to implement the same tricks to have some decent response times.

kjetilk commented 8 years ago

OK, now that you say it, I understand I'd have to subclass and extend it that way, OK.

But yeah, the LWP::UserAgent::CHICaching has its own tests for caching, so that's already taken care of. So, if that's sufficient for you, it should be OK.

The main way the caching could be improved is by storing a parsed response in the cache. I do that kind of thing in my module. There's already support for doing that in LWP::UserAgent::CHICaching, it can be implemented with a Role. However, since practical cache implementations are typically key-value and not triples, it is probably much work for mediocre gains. Another alternative is what I do in AtteanX::Query::Cache, where I assume that I have triples with at least one bound term, and so, the result will be an array or a hash. As for making it required, then perhaps we should just make LWP::UserAgent::CHICaching a prereq in the module metadata and the default? As of now, there are no other caching modules that conform to RFC7234 like LWP::UserAgent::CHICaching does, and I don't know if more flexibility will be useful.