spotify / dns-java

DNS wrapper library that provides SRV lookup functionality
Apache License 2.0
206 stars 47 forks source link

Added NoCachingLookupFactory #28

Closed jc89 closed 6 years ago

jc89 commented 7 years ago

NoCachingLookupFactory: LookupFactory not using Lookup cache (Setting it to null)

jc89 commented 7 years ago

@pettermahlen i think this could be useful when you are not interested in using the ttl declared in the records

I added a few test cases, will update the README later

pettermahlen commented 7 years ago

@jc89 looks good, the only thing left I think should be done is something like having the DnsSrvResolverBuilder throw an IllegalStateException if the user tries to set both useLookupCache and cacheLookups to true.

jc89 commented 7 years ago

@pettermahlen by default, previously it was basically using the Lookup cache always. In order to keep that behaviour, i set the default value for that caching flag in the lookup to true. If we add this validation, it could suddenly start breaking the build when people only update the version. We could:

pettermahlen commented 7 years ago

Sorry, I was confused. The problematic case is useLookupCache = false && cacheLookups = true. In this case, the useLookupCache configuration will be silently ignored, which I think would be confusing to the user.

Reading the above text, it's quite obvious that these two parameter names are very similar, and it's not clear what the difference between them really is. We should explain that somehow. Is it correct to say that: