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

Filter rcloud extensions, take 2 #1609

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

gordonwoodhull
Copy link
Contributor

This replaces #1361.

Currently we are only deactivating rcloud extensions for notebook calls. We should allow each session to specify, when it is initialized, which types of extensions should be loaded.

I have resurrected an old branch for this, which was not stable enough for 1.3. The idea is to have a set of flags in the DESCRIPTION file of each RCloud extension, which say what kinds of functionality the extensions provides. Then, after doing the usual per-instance and per-user filtering, these flags are intersected with the flags specified for session.init.

This is _not ready for merge_, since it introduces an unnecessary extra flag which was changing function signatures and wreaking havoc everywhere. As suggested by @s-u, we already have a flag for this, but that one needs to be upgraded to a vector of flags. So far I've just merged develop into this branch, so that we have an active PR for 1.4.3.

you can only .then() a function, not a promise
upgrading bluebird caught this
perhaps a poor name, but the idea is that when we initialize a session,
we say what kind of session we need, and only the appropriate extensions
are loaded
@gordonwoodhull gordonwoodhull added this to the 1.4.3 milestone Jul 30, 2015
@gordonwoodhull gordonwoodhull self-assigned this Aug 7, 2015
@gordonwoodhull
Copy link
Contributor Author

What @s-u and I discussed... a long time ago... was to use the existing mode, which is currently a 'scalar' string

  • change it into a vector of flag-strings
  • combine that functionality with the extra flags that are being sent along with the very breakful session_type in this PR
    Then the same flags will be used to determine whether to do compute separation and to decide which packages to load.

This means e.g. changing

    if (mode %in% cs.modes) { ## use fork only in modes that require it

https://github.com/att/rcloud/blob/develop/rcloud.support/R/ocaps.R#L88

to a union operation, etc etc. And removing most of the virulent interface changes in the current PR (yay).

@gordonwoodhull
Copy link
Contributor Author

I'll clean this up soon and pass back to @s-u for review.

gordonwoodhull added a commit that referenced this pull request Aug 14, 2015
can cause CSS problems, etc.
still not the extension-flags feature we want (#1609)
@gordonwoodhull
Copy link
Contributor Author

This seemed potentially too disruptive for the point release - could have strange effects for third-party extension developers, and we also need to make sure they know about the tags they need for the DESCRIPTION file. For now, we've disabled extensions also for mini.html (see commit above).

Hope to fix this up and merge to develop soon for 1.5.

@gordonwoodhull gordonwoodhull modified the milestones: 1.5, 1.4.3 Aug 16, 2015
@gordonwoodhull gordonwoodhull modified the milestones: 1.5, 1.6 Oct 16, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant