mengw15 commented on code in PR #7745:
URL: https://github.com/apache/texera/pull/7745#discussion_r3810775885
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResource.scala:
##########
@@ -53,16 +54,47 @@ object WarehouseResource {
name: String,
warehouseName: String,
flavor: String,
- createdAtMillis: Long
+ createdAtMillis: Long,
+ // Owner display info, mirroring DashboardWorkflowComputingUnit: today
every
+ // warehouse belongs to the caller, but the UI binds to the entry rather
than
+ // the session user so shared warehouses render the right person (#7743).
+ ownerName: String,
+ ownerAvatar: String
)
- private def toDashboardWarehouse(row: UserWarehouseRecord):
DashboardWarehouse =
+ // (name, avatar) per uid; null when the user has no name / avatar set,
matching
+ // how computing units resolve their owner info.
+ private def resolveOwners(uids: Seq[Integer]): Map[Integer, (String,
String)] =
+ if (uids.isEmpty) Map.empty
+ else
+ context
+ .select(USER.UID, USER.NAME, USER.AVATAR)
+ .from(USER)
+ .where(USER.UID.in(uids: _*))
+ .fetch()
Review Comment:
Right — I had hand-rolled a query for something `UserDao` already does.
Following the join below (your other comment), the listing no longer needs a
lookup at all, so the helper is now just the null-handling; `create` reads the
session user directly.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResource.scala:
##########
@@ -53,16 +54,47 @@ object WarehouseResource {
name: String,
warehouseName: String,
flavor: String,
- createdAtMillis: Long
+ createdAtMillis: Long,
+ // Owner display info, mirroring DashboardWorkflowComputingUnit: today
every
+ // warehouse belongs to the caller, but the UI binds to the entry rather
than
+ // the session user so shared warehouses render the right person (#7743).
+ ownerName: String,
+ ownerAvatar: String
)
- private def toDashboardWarehouse(row: UserWarehouseRecord):
DashboardWarehouse =
+ // (name, avatar) per uid; null when the user has no name / avatar set,
matching
+ // how computing units resolve their owner info.
+ private def resolveOwners(uids: Seq[Integer]): Map[Integer, (String,
String)] =
+ if (uids.isEmpty) Map.empty
+ else
+ context
+ .select(USER.UID, USER.NAME, USER.AVATAR)
+ .from(USER)
+ .where(USER.UID.in(uids: _*))
+ .fetch()
+ .asScala
+ .map(r =>
+ r.get(USER.UID) -> (
Review Comment:
Fixed in `efa9052`.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResource.scala:
##########
@@ -94,15 +126,18 @@ class WarehouseResource(client: LakekeeperClient, enabled:
Boolean) extends Lazy
if (!enabled) {
return WarehouseStatus(enabled = false, warehouses = List())
}
- val warehouses = context
+ val rows = context
.selectFrom(USER_WAREHOUSE)
.where(USER_WAREHOUSE.UID.eq(current_user.getUid))
.orderBy(USER_WAREHOUSE.CREATED_AT.asc())
.fetch()
- .map(row => toDashboardWarehouse(row))
+ .asScala
+ .toList
+ val owners = resolveOwners(rows.map(_.getUid).distinct)
Review Comment:
Done in `efa9052` — the listing is a single `leftJoin` on `USER` now, so one
round trip carries both the warehouses and their owners.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResource.scala:
##########
@@ -169,7 +204,7 @@ class WarehouseResource(client: LakekeeperClient, enabled:
Boolean) extends Lazy
}
throw new WebApplicationException(e.getMessage, 500)
}
- toDashboardWarehouse(row)
+ toDashboardWarehouse(row, resolveOwners(Seq(uid)).getOrElse(uid, (null,
null)))
Review Comment:
Good catch — the caller *is* the owner being resolved there, and
`SessionUser` already carries their name and avatar. Fixed in `efa9052`; no
query.
##########
amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/warehouse/WarehouseResource.scala:
##########
@@ -94,15 +126,18 @@ class WarehouseResource(client: LakekeeperClient, enabled:
Boolean) extends Lazy
if (!enabled) {
return WarehouseStatus(enabled = false, warehouses = List())
}
- val warehouses = context
+ val rows = context
.selectFrom(USER_WAREHOUSE)
.where(USER_WAREHOUSE.UID.eq(current_user.getUid))
.orderBy(USER_WAREHOUSE.CREATED_AT.asc())
.fetch()
- .map(row => toDashboardWarehouse(row))
+ .asScala
+ .toList
+ val owners = resolveOwners(rows.map(_.getUid).distinct)
WarehouseStatus(
enabled = true,
- warehouses = warehouses.toArray(Array[DashboardWarehouse]()).toList
+ warehouses =
+ rows.map(row => toDashboardWarehouse(row, owners.getOrElse(row.getUid,
(null, null))))
Review Comment:
It turned out to be removable rather than nameable: with the left join
supplying nulls directly and `create` reading the session user, nothing is left
that has to invent an absent owner, so the `(null, null)` fallback is gone in
`efa9052`. What remains is `ownerOf`, which only maps an unset name/avatar to
null.
--
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]
