Document that the default TTL is 60 seconds #37
Comments
stripethree
pushed a commit
to CondeNast/launch-vehicle-fbm
that referenced
this issue
Feb 14, 2017
## Why are we doing this? While investigating losing selfies #193 , it doth appear that [Cacheman's default TTL is 60 seconds](https://github.com/cayasso/cacheman/blob/6149f436268cedbec54633c81481ae7035411873/lib/index.js#L99-L104) but [it's not documented](cayasso/cacheman#37). So it was dropping data. This raises it to match our definition of a user session, one hour. ## Did you document your work? This explicitly sets the TTL, which increases documentations ## How can someone test these changes? Steps to manually verify the change: 1. Using the `master` branch 1. I started a bot 2. talked to the bot, noting the `count` log ``` lenses:messenger message.text user:1250872178269050 text: "fart" count: 1 seq: 8760 ``` 3. wait over a minute and talk again, the `count` will go back to `1`. 5. do the same thing with this branch, but the `count` will keep going up. ## What possible risks or adverse effects are there? I haven't looked into cacheman's internals, but this is effectively the same as a memory leak. We can transition to another backend if this is a problem. As it is, we're not using a lot of RAM. ## What are the follow-up tasks? * watch #193 ## Are there any known issues? none ## Did the test coverage decrease? same, initial values to cacheman are not part of the test suite.
crccheck
added a commit
to CondeNast/launch-vehicle-fbm
that referenced
this issue
Mar 13, 2017
## Why are we doing this? This is a copy paste of a fix that wasn't copy pasted into the initial SDK. Cacheman has a default TTL of [60 seconds](https://github.com/cayasso/cacheman/blob/6149f436268cedbec54633c81481ae7035411873/lib/index.js#L99-L104) but [it's not documented](cayasso/cacheman#37). This meant if you took a break in the middle of a quiz, it would freeze on you. ## Did you document your work? This explicitly sets the TTL, which increases documentation ## How can someone test these changes? Can't test within the SDK, but this is a copy paste ## What possible risks or adverse effects are there? * Uptime * Security * Performance degradation * Data loss If applicable, include a technical debt note. Do a quick estimate of if this pull request increases or decreases technical debt. Leave a message to future developers about why increased technical debt is worth it, or brag about how you figured out how to reduce technical debt. ## What are the follow-up tasks? * Make setting the session length a constructor option to the SDK ## Are there any known issues? None ## Did the test coverage decrease? same, initial values to cacheman are not part of the test suite.
|
IMO it should default to Number.MAX_VALUE. 😄 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The README currently doesn't say this
The text was updated successfully, but these errors were encountered: