Repository navigation
Conversation
9381467 to
593a2f0
Compare
Allow the Debug executor for the Aspire CLI run configuration. On Debug, `aspire run` is started with `ASPIRE_WAIT_FOR_DEBUGGER`, so the AppHost blocks in `DistributedApplication.CreateBuilder` until the .NET debugger attaches to it via the new `appHostDebugger` extension point. `DEBUG_SESSION_RUN_MODE` is set explicitly so child sessions are debugged only on Debug.
593a2f0 to
610f9c0
Compare
| ### Added | ||
|
|
||
| - Resource watching for the Aspire CLI run configuration | ||
| - Debugging of the AppHost launched by the Aspire CLI run configuration |
There was a problem hiding this comment.
It should be under the Unreleased section
| import kotlinx.coroutines.CoroutineScope | ||
| import java.nio.file.Path | ||
|
|
||
| interface AppHostDebuggerExtension { |
There was a problem hiding this comment.
@ApiStatus.Internal
| EP_NAME.extensionList.sortedBy { it.priority }.firstOrNull { it.isApplicable(appHostFilePath) } | ||
| } | ||
|
|
||
| val priority: Int |
There was a problem hiding this comment.
We probably don't need priority as we don't have multiple debugger for the same apphost type
|
|
||
| fun isApplicable(appHostFilePath: Path): Boolean | ||
|
|
||
| suspend fun attachToAppHost(appHostFilePath: Path, cliProcessPid: Long, project: Project, scope: CoroutineScope): Result<Unit> |
There was a problem hiding this comment.
Let's maybe make an ext function instead of the scope parameter: CoroutineScope.attachToAppHost(appHostFilePath: Path, cliProcessPid: Long, project: Project)
| const val DEBUG_SESSION_SERVER_CERTIFICATE = "DEBUG_SESSION_SERVER_CERTIFICATE" | ||
| const val DCP_INSTANCE_ID_PREFIX = "DCP_INSTANCE_ID_PREFIX" | ||
|
|
||
| // Launch mode the AppHost puts on the launch configurations of its DCP `run_session` requests. |
There was a problem hiding this comment.
Maybe this is an obvious comment, so we can remove it
| run.editor.open.browser=Open browser | ||
| run.editor.url=URL | ||
|
|
||
| configurable.Aspire=Aspire |
There was a problem hiding this comment.
Not sure about these configurable... strings. We already have them in the frontend part
| checkBox(AspireCoreFrontendBundle.message("configurable.Aspire.connect.to.dcp.via.https")) | ||
| .bindSelected(settings::connectToDcpViaHttps) | ||
| } | ||
| row { |
| checkBox(AspireCoreFrontendBundle.message("configurable.Aspire.show.app.host.not.debugged.notification")) | ||
| .bindSelected(settings::showAppHostNotDebuggedNotification) | ||
| } | ||
| row(AspireCoreFrontendBundle.message("configurable.Aspire.app.host.attach.timeout")) { |
There was a problem hiding this comment.
- Units (
secondshere) are ussually placed after the text field (see: https://plugins.jetbrains.com/docs/intellij/data-formats.html#units) - I think that the comment is redundant here. It almost says the same as the label.
- Probably, it is visually better to place text fields together and them the comboboxes.
| import kotlin.io.path.nameWithoutExtension | ||
| import kotlin.time.Duration.Companion.seconds | ||
|
|
||
| private val Path.pathWithoutExeOrDllExtension: Path |
There was a problem hiding this comment.
Let's move it to some util or extension class/file
| project: Project, | ||
| scope: CoroutineScope | ||
| ): Result<Unit> { | ||
| // Terminated when the run's scope is cancelled. |
There was a problem hiding this comment.
Probably the comment is obvious
| } | ||
|
|
||
| internal class DotNetAppHostDebuggerExtension : AppHostDebuggerExtension { | ||
| companion object { |
There was a problem hiding this comment.
A warning here: Companion objects in IDE extension implementations may only contain a logger and constants
| return project.solution.runnableProjectsModel.findBySessionProject(projectPath, runnablePredicate) | ||
| } | ||
|
|
||
| suspend fun findRunnableProjectByPathSuspending( |
There was a problem hiding this comment.
Let's remove the non-suspend variant
| if (outputPaths.isNullOrEmpty()) { | ||
| LOG.trace { "Unable to find the project output for $appHostFilePath" } | ||
| return emptyList() | ||
| } |
There was a problem hiding this comment.
We already log the warning in the parent attachToAppHost method if the outputs is empty. We probably don't need to log it here
|
|
||
| private suspend fun isAppHostProcess(process: EelProcessInfo, outputs: List<ProjectOutput>): Boolean { | ||
| val executable = process.executable ?: return false | ||
| val outputPaths = outputs.mapNotNull { it.exePath.toNioPathOrNull()?.pathWithoutExeOrDllExtension } |
There was a problem hiding this comment.
Let's create this in the parent findAppHostPid method and reuse for all the processes
| else -> this | ||
| } | ||
|
|
||
| internal class DotNetAppHostDebuggerExtension : AppHostDebuggerExtension { |
There was a problem hiding this comment.
I would suggest to implement this logic in a separate service and just call the service from the ext point. Services are more friendly for testing and reusing
| } | ||
|
|
||
| /** | ||
| * Returns the PID of the first descendant of [cliProcessPid] running the AppHost [outputs], or `null` when it |
There was a problem hiding this comment.
I believe this is clear too, so we don't need the comment
| } | ||
|
|
||
| private fun resolveAppHostDebugger(appHostFilePath: Path): AppHostDebuggerExtension? { | ||
| // Attaching to the AppHost process is local-only (the debugger discovers and attaches to a local |
There was a problem hiding this comment.
Attaching to the AppHost process is local-only -- is it?
| val project = environment.project | ||
|
|
||
| // Canceled on process termination, so a stop mid-attach cancels the polling cleanly. | ||
| val attachScope = AspireService.getInstance(project).scope.childScope("Aspire AppHost debugger attach") |
There was a problem hiding this comment.
I think that we shuould mark attachDebuggerToAppHost as suspend and reuse its scope instead of creating our own and managing it with ProcessListener. I believe that the parent suspend method will be cancelled if the target run configuration application couldn't start

No description provided.