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

State intent to solve problem of requiring assets #9

Merged
merged 1 commit into from Apr 4, 2016

Conversation

rgbkrk
Copy link
Member

@rgbkrk rgbkrk commented Apr 1, 2016

Libraries, and by extension kernels, have static assets that are required in frontends such as the notebook, thebe, hydrogen, and dashboards.

This issue continues to come up over and over without a total solution. At the very least I'd like for us to put it on the roadmap and acknowledge that we will solve it. My text description may not be the best, please let me know how to update/fix it.

Initial (old) proposal/discussion: jupyter/notebook#116
Related: jupyter/notebook#839

As it applies to Thebe, they currently have to bundle

  • all notebook js (not tied to kernel)
  • ipywidgets js (tied to kernel)
  • thebe itself

Each thebe release has to strictly specify the version of notebook, ipywidgets, in addition to thebe itself, and is incompatible with other versions.

Discussion from the Spring Jupyter Dev Meeting 2016: https://jupyter.hackpad.com/Spring-2016-Dev-Meeting-h0y1TIAWxz1#:h=Kernel-Static-Resources

/cc @parente @minrk @jdfreder @ellisonbg

Libraries, and by extension kernels, have static assets that are required in frontends such as the notebook, thebe, hydrogen, and dashboards..

This issue continues to come up over and over without a total solution. At the very least I'd like for us to put it on the roadmap and acknowledge that we will solve it.

Initial (old) proposal/discussion: jupyter/notebook#116
Related: jupyter/notebook#839

As it applies to Thebe, they currently have to bundle

* all notebook js (not tied to kernel)
* ipywidgets js (tied to kernel)
* thebe itself

Each thebe release has to strictly specify the version of notebook, ipywidgets, in addition to thebe itself, and is incompatible with other versions.

Discussion from the Spring Jupyter Dev Meeting 2016: https://jupyter.hackpad.com/Spring-2016-Dev-Meeting-h0y1TIAWxz1#:h=Kernel-Static-Resources
@rgbkrk
Copy link
Member Author

rgbkrk commented Apr 1, 2016

There's a wider group that could be CCed here, feel free to add people if you like. I didn't want to round up everyone all at once to prevent spamming everyone, even though I know this issue causes pain across much of the Jupyter ecosystem.

@parente
Copy link
Member

parente commented Apr 1, 2016

As it applies to Thebe, they currently have to bundle

jupyter-incubator/dashboard_server does not use Thebe, but suffers from similar library matching problems. We install a version of jupyter-js-widgets in the dashboard server and must ensure there is a compatible ipywidgets version installed in the kernel gateway providing the kernel for the dashboard. If the kernel-imported ipywidgets (or whatever widgets) could serve the appropriate paired web assets, we wouldn't have to bundle a version into the dashboard server and worry about incompatibilities.

(However, we would need to worry about jupyter/notebook#839 then.)

@sccolbert
Copy link

Following.

Should this scope also include installing/packaging/distributing the frontend code for extensions?

@parente
Copy link
Member

parente commented Apr 1, 2016

/cc @lbustelo

@rgbkrk
Copy link
Member Author

rgbkrk commented Apr 1, 2016

Should this scope also include installing/packaging/distributing the frontend code for extensions?

There's potential for this to creep towards it. I'd like to carve off a smaller problem here, which is assets per kernel.

@rgbkrk
Copy link
Member Author

rgbkrk commented Apr 1, 2016

To clarify, the extensions on JupyterLab have no place in Thebe, which gets used for inline execution on content like this: https://www.oreilly.com/learning/regex-golf-with-peter-norvig

@ellisonbg
Copy link
Contributor

I am +1 for merging this, but I think we need to leave roadmap PRs open longer for comments and make sure relevant people are pinged for feedback - especially after a busy week of the dev meeting where everyone is digging out from backlog. Also, because code isn't blocking on roadmap PRs, there isn't any hard to leaving them open longer than we normally would a code-based PR. Forming teams will help scope these discussions.

This particular PR is tiny and I am fine leaving it in, but want to make sure we are on the same page about discussions on roadmap PRs.

@fperez

@rgbkrk rgbkrk deleted the kernel-resources branch April 4, 2016 17:47
@rgbkrk
Copy link
Member Author

rgbkrk commented Apr 4, 2016

We can also revert this PR. People acknowledged it in person, complete with myriad solutions and we realized we could only make it forward with stating that we would solve it somehow. I'd like to start making an enhancement proposal for at least one way of doing this, seek feedback, etc.

What's the timeline that was agreed upon for merging these? Many others were merged as well in the last few days. What caught your attention with respect to this addition? When can people make a decision when as small as this and where the problem is recognized in many other issues?

/cc @Carreau

@ellisonbg
Copy link
Contributor

The roadmap process is brand new so we don't really have much of a
precedent. If there were other PRs in the roadmap merged, quickly I missed
it. In principle, the roadmap is one of the most important places for
mostly-independent subteams to communicate with the broader project and
have discussions.

Here is what I was thinking:

  • Work with Fernando and Jamie on the team survey
  • Based on that, ask subteams to gt up-to-date roadmaps for those portions
    of the project.
  • Leave them open long enough for the entire community to at least see and
    discuss them so there is the opportunity for cross team communication.

In this particular PR, it isn't the decision (I agree it is small) but
rather the communication process itself.

On Mon, Apr 4, 2016 at 10:59 AM, Kyle Kelley notifications@github.com
wrote:

We can also revert this PR. People acknowledged it in person, complete
with myriad solutions and we realized we could only make it forward with
stating that we would solve it somehow. I'd like to start making an
enhancement proposal for at least one way of doing this, seek feedback, etc.

What's the timeline that was agreed upon for merging these? Many others
were merged as well in the last few days. What caught your attention with
respect to this addition? When can people make a decision when as small as
this and where the problem is recognized in many other issues?

/cc @Carreau https://github.com/Carreau


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#9 (comment)

Brian E. Granger
Associate Professor of Physics and Data Science
Cal Poly State University, San Luis Obispo
@ellisonbg on Twitter and GitHub
bgranger@calpoly.edu and ellisonbg@gmail.com

@fperez
Copy link
Member

fperez commented Apr 5, 2016

Yup, no need to revert this, and for cases where we pretty much had already converged, I agree it's fine to go ahead.

But I do think it will be useful in the long run to have teams focus on the roadmap pieces that fall under their purview and do the more detailed screening. Then it's much easier for others (esp. me) to have a look at that stage, and merge/approve things that have been vetted on the details, focusing only on higher level integration across the whole project, match with our mission/vision/scope (which as we know, is everything :), etc.

@Carreau
Copy link
Member

Carreau commented Apr 5, 2016

Sorry if I merged some things too quickly. I don't see the roadmap either as something we should spend an eternity getting perfect.

Just this comment is longer than the all PR. I understand the need to discuss on some points, but I feel we are dangerously slipping into meeting to discuss how we decide the process to select the way collaborate in order to come up with a plan on how to achieve consensus.

@ellisonbg
Copy link
Contributor

I not asking for eternity or perfection, just leaving the PRs open long enough for individuals with different schedules, time zones and travel schedules to view them them and discuss if needed.

In an organization of this size and complexity some process and communications are needed. By merging almost immediately, with no discussion or comments, you made that difficult.

@Carreau
Copy link
Member

Carreau commented Apr 5, 2016

In an organization of this size and complexity some process and communications are needed. By merging almost immediately, with no discussion or comments, you made that difficult.

Brian, this PR was open for 3 days, and got no comments for 2.5 days. For a 2 line long sentence.
The last PR that was merged was open for 48 hours, and was literally typos. You cannot ask for everyone to +1 upper casing the first letter of a sentence.

  • last commit / merge commit
  • Apr 2 17:17 / Apr 4 10:53
  • Apr 1 13:56 / Apr 4 10:38
  • Apr 1 14:12 / Apr 2 15:37 (24 hours might be short but that's just copy and past of notes)

Asserting that things were merge immediately is inaccurate, not only for this PR, but for all the PR that were merge previously.

Most of the roadmap addition are just summary of what people discussed in person. I think we can trust each other to transcribe what you just discussed a few hours before.

Independently, but regarding one of your previous comments:

I disagree that leaving a dangling open PR is not annoying, when I look at the list of open PR on jupyter repos open roadmap PR take 1/2 of my screen, and contribute to 1/10 of the things I have to review.

So 10 time a days, there is 10 PR that can potentially be merged, where I have to wonder what to do with that, and this is time I'm loosing.

Personally I would prefer to get less PR in the queue and move things faster.

@takluyver
Copy link
Member

dwarf-kitten-01

Look at the kitten, everyone. Look at the kitten.

🐈

@rgbkrk
Copy link
Member Author

rgbkrk commented Apr 5, 2016

Personally I would prefer to get less PR in the queue and move things faster.

👍 GitHub's tooling tends to favor this as well. If I read a notification, it goes away until someone comments again. This is not the only org where I watch repos, so if I don't decide on an action or comment I'll miss out on contributing later.

@damianavila
Copy link
Member

People... let's iterate... this could be changed at any time if there is need.

Just this comment is longer than the all PR. I understand the need to discuss on some points, but I feel we are dangerously slipping into meeting to discuss how we decide the process to select the way collaborate in order to come up with a plan on how to achieve consensus.

This is funny... jeje

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 this pull request may close these issues.

None yet

9 participants