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

Restrict notebook communication topology #1812

Closed
wants to merge 1 commit into from

Conversation

Oblynx
Copy link

@Oblynx Oblynx commented Jan 7, 2022

Instigated by #300

In case Pluto wants this, some tests would have to change, because they verify that notebooks can talk to each other.
Maybe workers are intended to be able to talk to each other.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2022

Try this Pull Request!

Open Julia and type:

julia> import Pkg
julia> Pkg.activate(temp=true)
julia> Pkg.add(url="https://github.com/Oblynx/Pluto.jl", rev="restrict-distrib-comms")
julia> using Pluto

@Oblynx Oblynx mentioned this pull request Jan 7, 2022
@pankgeorg
Copy link
Collaborator

Hey!

I have two concerns

  1. this feature is listed as experimental
  2. it's not clear what we gain from this; Pluto does not make "security" promises at the moment so it feels limiting without a clear upside (We don't protect you from 'adversary notebooks' - notebooks are yours!)

Also I'm not sure if we actually need this to be open (@Pangoraw_ do you remember?)

Nevertheless, thank you for spending time on this! 💪🏾💪🏾

@Oblynx
Copy link
Author

Oblynx commented Jan 10, 2022

I agree with your concerns. I'd love to be able to use Distributed and would argue that Pluto has to support every part of Julia stdlib if it's to be a Julia IDE. Regarding notebook isolation though, I have no other reason to show this apart from Fons's comment in #300

@pankgeorg
Copy link
Collaborator

We are working on broad stdlib support! Latest major addition was the macro support 😄!
Until then we don't even claim to be Julia IDE 🙈🙈🙈 just simple reactive notebooks in Julia 🎈🎈🎈focused on education and teaching!

@Oblynx
Copy link
Author

Oblynx commented Jan 10, 2022

Latest major addition was the macro support

I can't express how important that is! Thank you so much for all your good work! I hope I can also help in a small way as a "power user" :)

@dralletje
Copy link
Collaborator

dralletje commented Jan 19, 2022

SHUT UP

This is literally all that is necessary to allow Distributed inside Pluto???
Also why can't workers talk to each other now?

  • Throws away Distributed clone 🙃 🥲

EDIT
Actually looked at the docs now.. awesome!
Being able to call code on the master process from the worker was a fun feature for tinkering around, but this is definitely what we want (as long as we don't sell it as "secure" because it ain't)

EDIT AGAIN
Okay got too excited, sadly doesn't make Distributed work.
I think this flag isn't too helpful (yet?), as it still allows worker->master eval even..

@Oblynx
Copy link
Author

Oblynx commented Jan 19, 2022

sadly doesn't make Distributed work

No this isn't about making Distributed work, for that check the last discussion in #300

as it still allows worker->master eval even

Really? Then I'm not sure if there's any point in this feature, maybe a bit of performance gain. Why don't you raise this up with Distributed upstream, to see what their use case is, since they mark this flag as "Experimental"

@dralletje dralletje closed this Feb 1, 2022
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.

3 participants