Improve update checking logic #226

Closed
Marak opened this Issue Apr 13, 2012 · 5 comments

Comments

Projects
None yet
5 participants
@Marak
Contributor

Marak commented Apr 13, 2012

Currently, jitsu will attempt to check for new versions.

see: https://github.com/nodejitsu/jitsu/blob/master/lib/jitsu/common/index.js#L140

This approach is not adequate. If you read the code, you'll see that we are checking npm with a 400 ms timeout every-time jitsu is run.

This is problematic for a couple reasons:

  1. In high latency situations ( or npm being slow ), this will always increase total time of jitsu commands by 400 ms.
  2. In the same situation, the update check will never happen ( see issue #223 )

We should periodically perform the update check, and probably move all this logic to Flatiron.

@jfhbrook

This comment has been minimized.

Show comment Hide comment
@jfhbrook

jfhbrook Sep 25, 2012

Contributor

Just checked up on this. The process is still exactly the same, though the code in question is now https://github.com/nodejitsu/jitsu/blob/master/lib/jitsu/common/index.js#L167-207 a few lines down.

I agree that this logic should be in a separate plugin, and that there has to be a smarter way to check for this.

@pksunkara Do you have any ideas for this? Want to give it a shot? Lemme know, we'll talk about it.

Contributor

jfhbrook commented Sep 25, 2012

Just checked up on this. The process is still exactly the same, though the code in question is now https://github.com/nodejitsu/jitsu/blob/master/lib/jitsu/common/index.js#L167-207 a few lines down.

I agree that this logic should be in a separate plugin, and that there has to be a smarter way to check for this.

@pksunkara Do you have any ideas for this? Want to give it a shot? Lemme know, we'll talk about it.

@pksunkara

This comment has been minimized.

Show comment Hide comment
@pksunkara

pksunkara Sep 25, 2012

Contributor

Have a config variable with the time when it was last checked and use it to check only once everyday?

Contributor

pksunkara commented Sep 25, 2012

Have a config variable with the time when it was last checked and use it to check only once everyday?

@jfhbrook

This comment has been minimized.

Show comment Hide comment
@jfhbrook

jfhbrook Sep 25, 2012

Contributor

Maybe. But what if the check once a day times out? Should jitsu keep trying every time? What if the internet is slow every time?

Maybe I'm overthinking this. Implementing a timed check is really not a bad idea.

Contributor

jfhbrook commented Sep 25, 2012

Maybe. But what if the check once a day times out? Should jitsu keep trying every time? What if the internet is slow every time?

Maybe I'm overthinking this. Implementing a timed check is really not a bad idea.

@yawnt

This comment has been minimized.

Show comment Hide comment
@yawnt

yawnt Sep 25, 2012

Contributor

you could overwrite the last checked variable only when the check is successful

Contributor

yawnt commented Sep 25, 2012

you could overwrite the last checked variable only when the check is successful

@blakmatrix

This comment has been minimized.

Show comment Hide comment
@blakmatrix

blakmatrix Mar 9, 2013

Contributor

A couple of comits ago the update feature was updated.

Contributor

blakmatrix commented Mar 9, 2013

A couple of comits ago the update feature was updated.

@blakmatrix blakmatrix closed this Mar 9, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment