add popup diaglog for missing IDE data on url handler #372

coryb · 2024-03-10T03:27:00Z

I really have no idea what I am doing, but this seems to work...

If the the jetbrains-gateway:// URL handler request is missing ide_product_code, ide_build_number, ide_path_on_host (or ide_download_link), or folder then we will open up a dialog that goes through the ide selection box (step 2 from normal UI flow).

Any feedback is most welcome!

coryb · 2024-03-10T03:27:33Z

gradle.properties

@@ -15,7 +15,7 @@ pluginUntilBuild=241.*
 # verifier should be used after bumping versions to ensure compatibility in the
 # range.
 platformType=GW
-platformVersion=233.6745-EAP-CANDIDATE-SNAPSHOT
+platformVersion=233.14808-EAP-CANDIDATE-SNAPSHOT


The previous version was expired, so could not run it locally.

src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt

coryb · 2024-03-10T03:30:37Z

src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt

+            } else
+                parameters.withProjectPath(parameters[FOLDER]!!)
+
+            params.withWorkspaceHostname(CoderCLIManager.getHostName(deploymentURL.toURL(), "${workspace.name}.${agent.name}"))


I am not sure how this ever worked ... this was just using the agent.name instead of ${workspace.name}.${agent.name} so it never matched any ssh config rules.

Good catch, I think I recently broke this because agent here used to be a WorkspaceAgentListModel and I have been refactoring to pass a regular WorkspaceAgent around, but I missed this name.

coryb · 2024-03-10T03:32:54Z

src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspaceStepView.kt

@@ -89,7 +89,7 @@ class CoderWorkspaceStepView(
    // Called with a boolean indicating whether IDE selection is complete.
    private val nextButtonEnabled: (Boolean) -> Unit,
 ) : CoderWizardStep {
-    private val cs = CoroutineScope(Dispatchers.Main)
+    private val cs = CoroutineScope(Dispatchers.IO)


I moved it to IO instead of Main because the app window seemed to be using Main so the IDE lazy-loaded selections were never running//loading. So I have no idea if this is correct or not...

@coryb is this code performing an I/O heavy operation, like waiting on network calls or disk read / writes? If not, Dispatchers.Unconfined might be okay. More info here: https://kotlinlang.org/docs/coroutine-context-and-dispatchers.html#dispatchers-and-threads

The operation I think uses ssh to connect to the workspace to look for installed ides, so it is certainly using network I/O, but probably does not qualify as "I/O heavy"

If that’s the case, the IO Dispatcher is likely safe to keep as-is 👍🏼

There probably is some magic to get Main working (for example we had a similar issue when launching from File > Remote Development and the fix was to add the modality state to launch), but this seems good to me! I had the general impression that updating UI from IO was a no-go (and here we update foreground colors, text, etc) but it seems to work fine as far as I can tell. 🤷

src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt

src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt

src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt

src/main/kotlin/com/coder/gateway/services/CoderSettingsState.kt

src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt

code-asher · 2024-03-11T19:40:31Z

src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt

+                val p = parameters.toMutableMap()
+                listOf(IDE_PRODUCT_CODE, IDE_BUILD_NUMBER, IDE_DOWNLOAD_LINK, IDE_PATH_ON_HOST, PROJECT_PATH).forEach { prop ->
+                    view.data()[prop]?.let { value -> p[prop] = value }
+                }
+                p


I think we can simplify this to:

Suggested change

val p = parameters.toMutableMap()

listOf(IDE_PRODUCT_CODE, IDE_BUILD_NUMBER, IDE_DOWNLOAD_LINK, IDE_PATH_ON_HOST, PROJECT_PATH).forEach { prop ->

view.data()[prop]?.let { value -> p[prop] = value }

}

p

parameters + view.data()

That said, I wonder if we want to always use the parameters from this step as-is rather than merging? I am not sure if maybe there is some weird combination that could cause issues. Say for example the link has IDE_PATH_ON_HOST for some reason but not the others, then from the workspace step we get IDE_DOWNLOAD_LINK and the product code and build number, maybe Gateway gets confused because it prefers the path if set, but then sees the code/build does not match (untested, so this is just thinking out loud).

So maybe we prefer paramaters and not overwrite with what we get back from the View. Then if one of IDE_PATH_ON_HOST or IDE_DOWNLOAD_LINK is set in params we skip setting those?

That works for me! I think at some point I want to refactor that step to only return the selected IDE data instead of the entire parameter set anyway, so this will be good in preparation for that, if I am following correctly.

Wait I think I misunderstood, if we ignore the path or link from the view and use whatever is in the original parameters we could still get a mismatch. If the original parameter link is http://jetbrains-221.tar.gz for example but in the step we selected 232 or some other version, we now have a map of "link" to "http://jetbrains-221.tar.gz", "code" to "232" which is odd.

Basically to be safe I think we have to always use all the IDE-related params from the original link, or all the IDE-related params from the step view, but never a combination of them.

Since the step view already returns a fully-specified set of parameters that connect to the workspace, we could just use them as-is for now to avoid any extra logic; like I mentioned above in the future maybe the step changes to only return IDE-related parameters but that could be dealt with at that time.

okay, makes sense, I put it back so we will prefer IDE_PRODUCT_CODE, IDE_BUILD_NUMBER, PROJECT_PATH, IDE_PATH_ON_HOST, IDE_DOWNLOAD_LINK returned from the view.

Thank you and sorry for all the back and forth. I think one of path or link from the step could be null which means we would end up using the value in the original parameters, but let me test that out when I add my changes and I will adjust if we need to!

code-asher · 2024-03-11T20:02:10Z

src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt

+            } else
+                parameters.withProjectPath(parameters[FOLDER]!!)
+
+            params.withWorkspaceHostname(CoderCLIManager.getHostName(deploymentURL.toURL(), "${workspace.name}.${agent.name}"))
                .withWebTerminalLink(client.url.withPath("/@$username/$workspace.name/terminal").toString())
                .withConfigDirectory(cli.coderConfigPath.toString())
                .withName(workspaceName)


Note to self: I think this is also meant to be in the workspace.agent format.

code-asher · 2024-03-11T21:26:15Z

src/main/kotlin/com/coder/gateway/views/steps/CoderWorkspaceStepView.kt

@@ -89,7 +89,7 @@ class CoderWorkspaceStepView(
    // Called with a boolean indicating whether IDE selection is complete.
    private val nextButtonEnabled: (Boolean) -> Unit,
 ) : CoderWizardStep {
-    private val cs = CoroutineScope(Dispatchers.Main)
+    private val cs = CoroutineScope(Dispatchers.IO)


There probably is some magic to get Main working (for example we had a similar issue when launching from File > Remote Development and the fix was to add the modality state to launch), but this seems good to me! I had the general impression that updating UI from IO was a no-go (and here we update foreground colors, text, etc) but it seems to work fine as far as I can tell. 🤷

code-asher

Thank you!!

coryb commented Mar 10, 2024

View reviewed changes

src/main/kotlin/com/coder/gateway/CoderGatewayConnectionProvider.kt Outdated Show resolved Hide resolved

coryb commented Mar 10, 2024

View reviewed changes

coryb force-pushed the url-handler-wizard branch from 52b5dc0 to ce8d268 Compare March 10, 2024 03:37

coryb commented Mar 10, 2024

View reviewed changes

src/main/kotlin/com/coder/gateway/cli/CoderCLIManager.kt Show resolved Hide resolved

coryb force-pushed the url-handler-wizard branch from ce8d268 to 1c0adab Compare March 10, 2024 21:28

code-asher reviewed Mar 11, 2024

View reviewed changes

coryb force-pushed the url-handler-wizard branch from 1c0adab to e7f9467 Compare March 11, 2024 23:34

add popup dialog for missing data on url handler

f19588f

coryb force-pushed the url-handler-wizard branch from e7f9467 to f19588f Compare March 11, 2024 23:42

code-asher approved these changes Mar 11, 2024

View reviewed changes

code-asher merged commit 76d298b into coder:main Mar 12, 2024

matifali mentioned this pull request May 8, 2025

JetBrains-Gatway: Show a version picker and folder picker if they are not provided coder/registry#70

Open

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add popup diaglog for missing IDE data on url handler #372

add popup diaglog for missing IDE data on url handler #372

coryb commented Mar 10, 2024

coryb Mar 10, 2024

coryb Mar 10, 2024

code-asher Mar 11, 2024

coryb Mar 10, 2024

benduran Mar 10, 2024

coryb Mar 10, 2024

benduran Mar 10, 2024

code-asher Mar 11, 2024

code-asher Mar 11, 2024 •

edited

Loading

coryb Mar 11, 2024

code-asher Mar 11, 2024 •

edited

Loading

code-asher Mar 11, 2024 •

edited

Loading

coryb Mar 11, 2024

code-asher Mar 11, 2024 •

edited

Loading

code-asher Mar 11, 2024

code-asher Mar 11, 2024

code-asher left a comment

add popup diaglog for missing IDE data on url handler #372

add popup diaglog for missing IDE data on url handler #372

Conversation

coryb commented Mar 10, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

code-asher Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

code-asher Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

code-asher Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

code-asher Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

code-asher left a comment

Choose a reason for hiding this comment

code-asher Mar 11, 2024 •

edited

Loading

code-asher Mar 11, 2024 •

edited

Loading

code-asher Mar 11, 2024 •

edited

Loading

code-asher Mar 11, 2024 •

edited

Loading