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

Feature idea: make the "node" attr of the Remote Execution smart cell accept a variable name #455

Open
hugobarauna opened this issue Jul 9, 2024 · 8 comments

Comments

@hugobarauna
Copy link
Member

hugobarauna commented Jul 9, 2024

Imagine a scenario where

  • the user has a Phoenix app running clustered
  • using the default DNS cluster from Phoenix for cluster formation
  • and the IPs of those Phoenix/elixir instances are dynamic

Now, If the user wants to use the remote execution smart cell to call code from that Phoenix app, they could first get the FQDN of one of those Phoenix/Elixir instances:

service_name = "app_server"
dns_query = "app_server"

ip =
  :inet_res.getbyname(~c"#{dns_query}", :a)
  |> then(fn {:ok, {_, _, _, _, _, ips}} -> ips end)
  |> List.first()
  |> Tuple.to_list()
  |> Enum.join(".")

fqdn = :"#{service_name}@#{ip}"

Then, they could use the remote smart cell to call code from the Phoenix app:

CleanShot 2024-07-09 at 19 03 35

But, the smart cell doesn't accept a variable name for the node attribute.

So the only solution is to convert the smart cell into code and go from there. Ok, it's not ideal, but it works:

CleanShot 2024-07-09 at 19 02 23

However, the problem happens if, in the future, when continuing to work on the notebook, the user wants to edit the code that is called inside the smart cell. Now, they have lost the DX of editing code inside the smart cell (syntax highlight, code completion, etc.).

So, my idea is to make the smart cell accept a variable name as the node attribute in addition to what it already accepts (a string literal or a Livebook secret).

I'd like to know if we think it makes sense to work on that.

@josevalim
Copy link
Contributor

josevalim commented Jul 9, 2024

Actually, I think the node name has to be static for us to leverage code completion, unless we add a mechanism for smart cells to update their attributes. So I think it could be done, but there will be technical challenges.

@jonatanklosko
Copy link
Member

@josevalim we already have an API for changing the editor intellisense node as of #390!

Extending the field to accept variable sounds good to me. We probably should make it a select as we do in such cases. When scanning the variables we can filter atoms specifically, for node names we could additionally check if the atom contains @.

Note that ideally we want to maintain compatibility with other notebooks, so on init we need to map the old attributes to whatever new format we choose. Currently we have use_node_secret: boolean, node, node_secret, we could for example change it to node_source: "text" | "secret" | "variable" and store the value in node_{node_source}.

@bradleygolden
Copy link

👋, just want to give my support for this feature. 😊

We're currently ramping up use of livebook teams across our engineering team and ran into this issue.

@hugobarauna outlined our problem perfectly. We have a Phoenix app deployed across multiple environments in AWS and are using libcluster's DNS adapter. Our ideal workflow would be to write livebook notebooks locally and then deploy them to remote environments with the option to execute remote functions via the remote execution smart cell. The current solution of converting the smart cell to code works but definitely makes things more complicated.

In general, I find the use case of allowing variables in smart cells very useful so they can be used in environments where inputs can change. The sql smart cell is another example of this, but it's simpler so converting that cell to code isn't as big of a deal for us.

@josevalim
Copy link
Contributor

@bradleygolden for the SQL smart cell, wouldn't secrets be enough? Or do you also have a moving host/ip address?

@bradleygolden
Copy link

Secrets would be enough

@josevalim
Copy link
Contributor

@bradleygolden which fields do you want to customize? Are they not secrets-enabled today? :)

@bradleygolden
Copy link

Oh apologies @josevalim! I meant to say that our database hostname when developing locally is localhost but when it's deployed it's something like instance.abc123.us-west-1.rds.amazonaws.com, so in that sense you can say it's moving. In reality I think it's safe to assume it could be different by environment.

@bradleygolden
Copy link

bradleygolden commented Aug 7, 2024

Thinking about this some more actually, the secrets are also rotating so we'd need those to constantly be updating via a variable rather than a secret itself since secrets can't be modified in deployed livebook apps. Maybe using the kino db code directly would be better than using the SQL smart cell. 😄

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

No branches or pull requests

4 participants