PG1204 commented on code in PR #6446:
URL: https://github.com/apache/texera/pull/6446#discussion_r3598369989
##########
computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala:
##########
@@ -152,18 +152,22 @@ class ComputingUnitAccessResource {
}
// TODO: add try except and check how to display error message in the
frontend
- val granteeId = userDao.fetchOneByEmail(email).getUid
- if (granteeId == null) {
+ val grantee = userDao.fetchOneByEmail(email)
+ if (grantee == null) {
throw new IllegalArgumentException("User with the given email does not
exist")
Review Comment:
thanks for the review, this is exactly right, I confirmed the
IllegalArgumentException was falling through to Dropwizard's default
LoggingExceptionMapper and surfacing as an opaque 500. Switched to JAX-RS
exceptions: BadRequestException (400 + message) for the unknown-email case, and
ForbiddenException (403) for the permission checks, I went with 403 there for
the semantic status and to match the existing
DatasetAccessResource/WorkflowAccessResource convention. The tests now
intercept the JAX-RS types and assert the resolved status (400/403).
addressed in
https://github.com/apache/texera/commit/91ad863aa74374eb5a280d890fd822cc5720938f
--
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]
