This is really dope, thanks for sharing!

Enjoyed the video :)

That sounds good to me. What name did you have in mind for the config key?

I think it should be fully qualified, but that's all I got

:portal.server/port ?

Yeah, that sounds good. Or :portal.expose/port

Or trade expose for ui

I think that could work. What other keys could you see under that ns?

host in case not accessed over local host. And maybe launch-command for specifying something other than chrome.

Those are the only that come to mind.

I think I might like :portal.launcher/port :portal.launcher/host and :portal.launcher/command then 👌

Those sound great.

I don't think we need to implement them all right now, :portal.launcher/port alone would be an awesome add!

Let me know if you need any help with the code

Cool. Thank you.

I might have a similar problem with an upcoming feature for babashka pods: instead of stdin/stdout pods can choose to use a socket for communication and by default, the pod will choose a random open port (this is for pods that want to have full control over the terminal, e.g. TUI)

https://github.com/babashka/pod-babashka-lanterna/blob/96e2c4571594956c8e709c602137fbb989ca507a/lanterna.cljc#L6

@U0510902N random ports usually go in the higher area > 10000 I think? it would be highly co-incidental if that would conflict with one of your pre-configured ssh tunnel ports?

The problem I run into isn't that portal's ports conflict with my already-forwarded ports, but that in order to access the portal port, I have to make a new port forward. When I can specify the portal port, I'll set my forwarding to include a few well chosen ports and just use one of those when opening portal.

Your idea about using a socket for pod communication sounds really cool. As does a lanterna pod.

Ah right. So there's no need for me to make the port of the pod configurable then I guess?

The lanterna pod can already run console-tetris

Yes, indeed.

@U1G869VNV PR opened. Had a couple questions too in the comments https://github.com/djblue/portal/pull/19

@U0510902N to your second question, maybe we can have a lower level api that is only available for server backed rpc implementations. Like first you (portal.launcher/start host port) , then you (portal.api/open) as many times as you like? I think it's simpler but definitely not as easy 😬

ah, that makes a lot of sense

so if you call (portal.api/open), you get the default behavior and if you want to specify the host/port, then you need to run (portal.launcher/start host port) first?

That's my thinking, would that be reasonable?

I think so

and would this only be necessary for the jvm and node launchers?

I guess it could also be portal.api/start

With a description about how it's a low level thing that you only need to care about if you want control over the server

yeah, portal.api/start sounds good

I'll update the PR with this later

Sounds great, thanks!

you're welcome, thank you for the feedback

@U1G869VNV I was working on this yesterday morning and realized that only port is unchangeable (and should be passed to the start function). host is something that can be different on each open call. Is it confusing to have :portal.colors/theme and :portal.launcher/host as options for open and only honor :portal.launcher/port in start?

the tricky part about making start the way to override host is that it's not an attribute of the server, so I would need to store it in the atom as well so that open can use it

I'm leaning toward host and port being just for start, because that seems less confusing, thoughts?

@U0510902N I agree, keeping those options for start now seems best

cool, thank you

Apologies on the delay in getting this done, work rather exploded this week. I should have some time on Saturday to get the PR updated.

@U1G869VNV I just pushed a commit that separates out the a new start function. I'm sure of the JVM launcher code. I'm less sure of the Node launcher code as I was only able to test it by running the node sample (I couldn't connect my editor to the repl and start/stop, but I was able to stop via the actual REPL prompt). Please let me know what you think of the code and if there's anything I should change. Thank you.

@U0510902N, to test different environments, I use make e2e . It looks like you might have missed fixing p/close for node. You can run make e2e/node specifically to run those tests.

The main issues I'm seeing is that the node process isn't terminating which probably means the server is still running.

diff --git a/src/portal/runtime/node/launcher.cljs b/src/portal/runtime/node/launcher.cljs
index 05f89fe..80926b9 100644
--- a/src/portal/runtime/node/launcher.cljs
+++ b/src/portal/runtime/node/launcher.cljs
@@ -47,14 +47,12 @@
 
 (defn start [options]
   (when (nil? @server)
-    (reset!
-     server
-     (a/let [{:portal.launcher/keys [port host] :or {port 0 host "localhost"}} options
-             server (create-server #'server/handler port host)]
-       server))))
+    (a/let [{:portal.launcher/keys [port host] :or {port 0 host "localhost"}} options
+            instance (create-server #'server/handler port host)]
+      (reset! server instance))))
 
 (defn- stop [handle]
-  (some-> handle :http-server (.close)))
+  (some-> handle :http-server .close))
 
 (defn open [options]
   (let [session-id (random-uuid)]

@U0510902N ☝️ should fix it. a/let returns a promise

Ah! Ok. Thanks for that. I'll add that and run the e2e tests.

@U1G869VNV I just pushed up the above code change as a commit. Looks like the jvm, bb, and node e2e tests run well. I can't get e2e/web to work.

You might have to enable pop-ups but it's fine, everything looks good to me locally.

@U0510902N I just merged the PR, thanks! I should have a release out in the next couple of days.

awesome, thank you for merging

and thank you for your feedback during the process