livebook-dev / kino

Client-driven interactive widgets for Livebook
Apache License 2.0
358 stars 60 forks source link

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

Open hugobarauna opened 1 month ago

hugobarauna commented 1 month ago

Imagine a scenario where

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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

👋, 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 commented 1 month ago

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

bradleygolden commented 1 month ago

Secrets would be enough

josevalim commented 1 month ago

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

bradleygolden commented 3 weeks ago

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 commented 3 weeks ago

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. 😄