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

Cache TTL doesn't seem to work #12

Closed
bpb27 opened this issue Feb 28, 2019 · 4 comments · Fixed by #13 or #16
Closed

Cache TTL doesn't seem to work #12

bpb27 opened this issue Feb 28, 2019 · 4 comments · Fixed by #13 or #16

Comments

@bpb27
Copy link

bpb27 commented Feb 28, 2019

Hey thanks for this package!

I'm running v0.1.4 and noticed that when using getBatchedAndCached, passing a TTL doesn't do anything, and the set value seems to live in the cache forever. I tried:

getBatchedAndCached(query, 1);
getBatchedAndCached(query, 0);
getBatchedAndCached(query, -1);
getBatchedAndCached(query, { ttl: 0 });

I think the issue might be that apollo-server-caching@0.1.2 doesn't actually use a TTL in set.

class InMemoryLRUCache {
  constructor({ maxSize = Infinity } = {}) {
    this.store = LRU({ max: maxSize, length: item => item.length });
  }
  get(key) {
    return __awaiter(this, void 0, void 0, function* () {
      return this.store.get(key);
    });
  }
  set(key, value) {
    return __awaiter(this, void 0, void 0, function* () {
      this.store.set(key, value);
    });
  }
}

I can get around it by installing apollo-server-caching@0.3.1 and passing in { cache: new InMemoryLRUCache() } in initialize.

Not sure if I'm missing something, but I think you can just update the dependency.

@cvburgess
Copy link
Owner

Thank you so much! This has been bugging me but I have not had time to dive in.

cvburgess added a commit that referenced this issue Feb 28, 2019
cvburgess added a commit that referenced this issue Feb 28, 2019
@cvburgess
Copy link
Owner

https://github.com/cvburgess/SQLDataSource/releases/tag/v0.1.5 Should resolve the cache TTL issue.

CC: @tab00

@bpb27
Copy link
Author

bpb27 commented Feb 28, 2019

Thanks! Confirmed that it's fixed. 🎉

Might want to update the README example:

const MINUTE = { ttl: 60 };

The Apollo package looks for a ttl prop in an options object and multiplies the number by 1000.

@cvburgess
Copy link
Owner

Oh thats great to know - would you mind opening a PR for that?

@cvburgess cvburgess pinned this issue Mar 1, 2019
@cvburgess cvburgess unpinned this issue Apr 3, 2019
@cvburgess cvburgess mentioned this issue Apr 5, 2019
cvburgess added a commit that referenced this issue Apr 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants