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

add activity watching to kernels #1827

Merged
merged 9 commits into from Jan 24, 2017

Conversation

8 participants
@minrk
Member

minrk commented Oct 11, 2016

adds to kernel REST model:

  • last_activity: datetime-aware timestamp
  • execution_state: from last recorded status message

related: jupyterlab/jupyterlab#1070

next PR: shutdown idle kernels

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Oct 11, 2016

Member

need to update a few tests to take the added timestamps into account

Member

minrk commented Oct 11, 2016

need to update a few tests to take the added timestamps into account

@parente

This comment has been minimized.

Show comment
Hide comment
@parente

parente Oct 11, 2016

Member

I just spotted this in the hackpad. Does it align with the activity tracking added to the kernel gateway a while back?

https://github.com/jupyter/kernel_gateway/blob/master/kernel_gateway/jupyter_websocket/swagger.yaml#L298

Member

parente commented Oct 11, 2016

I just spotted this in the hackpad. Does it align with the activity tracking added to the kernel gateway a while back?

https://github.com/jupyter/kernel_gateway/blob/master/kernel_gateway/jupyter_websocket/swagger.yaml#L298

last_activity:
type: string
description: |
ISO 8601 timestamp for the last-seen activity on this kernel.

This comment has been minimized.

@parente

parente Oct 11, 2016

Member

Should this explicitly be UTC?

@parente

parente Oct 11, 2016

Member

Should this explicitly be UTC?

This comment has been minimized.

@minrk

minrk Oct 12, 2016

Member

They are explicitly timezone-aware, and UTC as an implementation detail. I'll add that detail, though.

@minrk

minrk Oct 12, 2016

Member

They are explicitly timezone-aware, and UTC as an implementation detail. I'll add that detail, though.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Oct 12, 2016

Member

@parente I forgot about the KG activity API! This is significantly less than that:

  1. it only tracks a single activity stamp, not categories
  2. it's two added fields (last_activity, execution_state) in the kernels API, rather than a new API endpoint

Do you think we should pull in the KG API wholesale, rather than this simpler tracker? What have you used the more detailed information for?

Member

minrk commented Oct 12, 2016

@parente I forgot about the KG activity API! This is significantly less than that:

  1. it only tracks a single activity stamp, not categories
  2. it's two added fields (last_activity, execution_state) in the kernels API, rather than a new API endpoint

Do you think we should pull in the KG API wholesale, rather than this simpler tracker? What have you used the more detailed information for?

@parente

This comment has been minimized.

Show comment
Hide comment
@parente

parente Oct 12, 2016

Member

Do you think we should pull in the KG API wholesale, rather than this simpler tracker? What have you used the more detailed information for?

I think the extra detail was added shotgun style because we didn't have our hands around everything that operators would want to know to make decisions about monitoring and shutting down kernels. At a minimum it'd be good to have the field names and types that are common match at least, even if the API is not 100% the same.

The original issue is here: jupyter/kernel_gateway#96

@rwhorman probably has more detail on what's used or not at the moment.

Member

parente commented Oct 12, 2016

Do you think we should pull in the KG API wholesale, rather than this simpler tracker? What have you used the more detailed information for?

I think the extra detail was added shotgun style because we didn't have our hands around everything that operators would want to know to make decisions about monitoring and shutting down kernels. At a minimum it'd be good to have the field names and types that are common match at least, even if the API is not 100% the same.

The original issue is here: jupyter/kernel_gateway#96

@rwhorman probably has more detail on what's used or not at the moment.

@yuvipanda

This comment has been minimized.

Show comment
Hide comment
@yuvipanda

yuvipanda Oct 24, 2016

Member

\o/ Can we use this from the hub too, thus taking out last activity tracking from the proxy?

Member

yuvipanda commented Oct 24, 2016

\o/ Can we use this from the hub too, thus taking out last activity tracking from the proxy?

@rgbkrk

This comment has been minimized.

Show comment
Hide comment
@rgbkrk

rgbkrk Oct 24, 2016

Member

This is so excellent to see.

Member

rgbkrk commented Oct 24, 2016

This is so excellent to see.

@yuvipanda

This comment has been minimized.

Show comment
Hide comment
@yuvipanda

yuvipanda Nov 10, 2016

Member

jupyterhub/jupyterhub#857 is related, talks about adopting a standardized format for exposing white-box metrics (such as last-activity!).

Member

yuvipanda commented Nov 10, 2016

jupyterhub/jupyterhub#857 is related, talks about adopting a standardized format for exposing white-box metrics (such as last-activity!).

@parente

This comment has been minimized.

Show comment
Hide comment
@parente

parente Nov 22, 2016

Member

Haven't heard back from @rwhorman on the comment above.

@jtyberg @lbustelo do either of you guys know if the activity tracking API proposed here vs the one with way more properties in the kernel gateway will satisfy existing use cases?

Member

parente commented Nov 22, 2016

Haven't heard back from @rwhorman on the comment above.

@jtyberg @lbustelo do either of you guys know if the activity tracking API proposed here vs the one with way more properties in the kernel gateway will satisfy existing use cases?

@enigmata

This comment has been minimized.

Show comment
Hide comment
@enigmata

enigmata Nov 23, 2016

@parente Nothing conclusive, as we're still experimenting and adjusting our model.

enigmata commented Nov 23, 2016

@parente Nothing conclusive, as we're still experimenting and adjusting our model.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Jan 13, 2017

Member

Heads up, this has a merge conflict.

Member

takluyver commented Jan 13, 2017

Heads up, this has a merge conflict.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Jan 13, 2017

Member

Thanks, rebased.

Member

minrk commented Jan 13, 2017

Thanks, rebased.

@yuvipanda

This comment has been minimized.

Show comment
Hide comment
@yuvipanda

yuvipanda Jan 13, 2017

Member
Member

yuvipanda commented Jan 13, 2017

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Jan 13, 2017

Member

I'm hoping we might go straight to 5.0, because I'm getting tired of telling people that the new clipboard functionality (#1286) is not available yet.

Member

takluyver commented Jan 13, 2017

I'm hoping we might go straight to 5.0, because I'm getting tired of telling people that the new clipboard functionality (#1286) is not available yet.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver Jan 20, 2017

Member

What's the status of this? I'm trying to push towards a 5.0 release. Is this going to be mergeable fairly soon, or should we hold it for 5.1?

Member

takluyver commented Jan 20, 2017

What's the status of this? I'm trying to push towards a 5.0 release. Is this going to be mergeable fairly soon, or should we hold it for 5.1?

minrk added some commits Oct 11, 2016

add activity watching to kernels
adds to kernel REST model:

- last_activity: datetime-aware timestamp
- execution_state: from last recorded status message
track REST API activity
add /api/status endpoint for retrieving current status

includes

- started: start time of the server
- last_activity: latest activity across all endpoints
- connections: number of current connections
- kernels: number of current kernels
@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Jan 20, 2017

Member

@takluyver fixing it now. I think it's in good shape, I just need to update the new tests to deal with the token changes.

Member

minrk commented Jan 20, 2017

@takluyver fixing it now. I think it's in good shape, I just need to update the new tests to deal with the token changes.

minrk added some commits Jan 20, 2017

@ivanov

This comment has been minimized.

Show comment
Hide comment
@ivanov

ivanov Jan 21, 2017

Member

The js/tree group for py3.5.1 build was stuck on Travis for 2 hours and 45 minutes without starting, so I stopped it and restarted it just now.

Member

ivanov commented Jan 21, 2017

The js/tree group for py3.5.1 build was stuck on Travis for 2 hours and 45 minutes without starting, so I stopped it and restarted it just now.

@minrk

This comment has been minimized.

Show comment
Hide comment
@minrk

minrk Jan 21, 2017

Member

Thanks, @ivanov! Looks all green now.

I'm happy with it as-is now, but further reviews and ideas are always welcome.

Member

minrk commented Jan 21, 2017

Thanks, @ivanov! Looks all green now.

I'm happy with it as-is now, but further reviews and ideas are always welcome.

@rgbkrk

rgbkrk approved these changes Jan 21, 2017

@takluyver takluyver merged commit f5072cf into jupyter:master Jan 24, 2017

4 checks passed

codecov/patch 99.33% of diff hit (target 0.00%)
Details
codecov/project 76.76% (+1.83%) compared to 58c66ef
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@blink1073

This comment has been minimized.

Show comment
Hide comment
@blink1073

blink1073 Jan 24, 2017

Member

🎉

Member

blink1073 commented Jan 24, 2017

🎉

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