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

no support for manually setting ttl in set method #7

Open
harshitj2005 opened this issue May 4, 2017 · 3 comments
Open

no support for manually setting ttl in set method #7

harshitj2005 opened this issue May 4, 2017 · 3 comments

Comments

@harshitj2005
Copy link

harshitj2005 commented May 4, 2017

hi,

i have been using this libray for a while now. but now i want to set my own ttl in set method i:e not using session maxage but there is no support for it in your libray. i have tried some code and seems to be working. i also have tried to update the same on the repo but not authorised. please have a look on the code

MemcachedStore.prototype.set = function(sid, sess, fn,maxAgettl) {
      sid = this.getKey(sid);

      try {
          var defaultCokie = { httpOnly: true, secure : false, maxAge : (4 * 60 * 60)};
          var cookie = sess.cookie && sess.cookie||defaultCokie;
          var maxAge = maxAgettl && maxAgettl||cookie.maxAge;
          var ttl = 'number' == typeof maxAge ? maxAge | 0 : oneDay
          var sess = JSON.stringify(sess);

          this.client.set(sid, sess, function() {
              fn && fn.apply(this, arguments);
          }, ttl);
      } catch (err) {
          fn && fn(err);
      } 
  };
@liamdon
Copy link
Owner

liamdon commented Jul 19, 2017

I'm happy to add this as an option but I'm never a fan of the optional arguments after the callback fn. I'd rather add a second function like setTTL or setex - more like how redis does it.

What do you think?

@harshitj2005
Copy link
Author

well setTTL looks fine.
there is one more concern now. memjs library started giving warning

MemJS SET: using deprecated call - arguments have changed

@saschat
Copy link

saschat commented Apr 4, 2018

This warning is because the set command arguments changed to follow Node.js conventions. So

this.client.set(sid, sess, function() {
    //...
}, ttl);

should now be

this.client.set(sid, sess, {'expires': ttl}, function() {
    //...
});

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

No branches or pull requests

3 participants