mengw15 commented on code in PR #8586:
URL: https://github.com/apache/texera/pull/8586#discussion_r4050035864
##########
amber/src/main/scala/org/apache/texera/web/resource/SyncExecutionResource.scala:
##########
@@ -169,7 +174,7 @@ class SyncExecutionResource extends LazyLogging {
),
emailNotificationEnabled = false,
computingUnitId = computingUnitId,
- warehouseId = None
+ warehouseId = request.warehouseId
Review Comment:
Right — the hoist only covered `initExecutionService`, and this endpoint
tears the previous execution down before ever reaching it, so the agent path
still had the problem Kun described. The pick is resolved before
`shutdownPreviousExecution` now. It is resolved again inside the init call: one
indexed single-row read on a request that already writes several rows, which
seemed a better trade than threading the resolved value through a signature
shared with the websocket caller.
##########
amber/src/main/scala/org/apache/texera/web/service/WorkflowService.scala:
##########
@@ -219,21 +230,24 @@ class WorkflowService(
sessionUri: URI
): Unit = {
- if (executionService.hasValue) {
- executionService.getValue.unsubscribeAll()
- }
-
val (uidOpt, userEmailOpt) = userOpt.map(user => (user.getUid,
user.getEmail)).unzip
+ // Validate before touching the execution already in flight: a request
that is
+ // going to be refused must not take the running one's subscriptions with
it.
// uid is NOT NULL in the DB; fail early here rather than letting the
insert fail downstream.
val uid = uidOpt.getOrElse(
throw new IllegalArgumentException(
"Cannot start execution: a user id (uid) is required but none was
provided."
)
)
+ val warehouseName =
WorkflowService.resolveLakekeeperWarehouseName(req.warehouseId, uid)
+
+ if (executionService.hasValue) {
+ executionService.getValue.unsubscribeAll()
+ }
Review Comment:
Fixed — that test asserted the old contract and was the amber CI failure. It
now asserts that a refused request leaves the previous execution attached.
Worth noting: the uid case alone could not pin the new ordering (it already
threw before the teardown), so there is a new case that fails resolution on the
warehouse itself.
##########
agent-service/src/server.ts:
##########
@@ -492,6 +496,13 @@ export function buildApp() {
wsLog.info({ agentId, preview: msg.content.substring(0, 50) },
"received command");
+ // The prompt carries the workspace's current warehouse pick, so a
run
+ // uses what the user has selected now rather than whatever was
+ // selected when the agent was created (#7751).
+ if (typeof msg.warehouseId === "number") {
+ agent.setDelegateWarehouse(msg.warehouseId);
+ }
Review Comment:
Good catch — an absent field is the selection it represents, so it now
clears the previous pick instead of leaving a stale id to be sent and refused
once the feature is off.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
