[jira] [Commented] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15898150#comment-15898150
]
Hadoop QA commented on SENTRY-1546:
---
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12856342/SENTRY-1546.005.patch
against master.
{color:green}Overall:{color} +1 all checks pass
{color:green}SUCCESS:{color} all tests passed
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/2405/console
This message is automatically generated.
> Generic Policy provides bad error messages for Sentry exceptions
>
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
>Affects Versions: 1.8.0, sentry-ha-redesign
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch, SENTRY-1546.005.patch
>
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response respose = requestHandle(new RequestHandler() {
> @Override
> public Response handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15883718#comment-15883718
]
Hadoop QA commented on SENTRY-1546:
---
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12854601/SENTRY-1546.003.patch
against master.
{color:red}Overall:{color} -1 due to 2 errors
{color:red}ERROR:{color} mvn test exited 1
{color:red}ERROR:{color} Failed:
org.apache.sentry.provider.db.generic.service.persistent.TestPrivilegeOperatePersistence
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/2368/console
This message is automatically generated.
> Generic Policy provides bad error messages for Sentry exceptions
>
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch
>
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response respose = requestHandle(new RequestHandler() {
> @Override
> public Response handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
[jira] [Commented] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15826889#comment-15826889
]
kalyan kumar kalvagadda commented on SENTRY-1546:
-
It makes sense. I'm not sure if it is feasible in all the cases.
> Generic Policy provides bad error messages for Sentry exceptions
>
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch
>
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response respose = requestHandle(new RequestHandler() {
> @Override
> public Response handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15826848#comment-15826848
]
Alexander Kolbasov commented on SENTRY-1546:
Since we want a unified message in such cases would it make sense to modify
constructors for SentryNoSuchObjectException and SentryAlreadyExistsException
to provide such uniform messages for us so that callers don't have to hunt for
proper message format?
> Generic Policy provides bad error messages for Sentry exceptions
>
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch
>
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response respose = requestHandle(new RequestHandler() {
> @Override
> public Response handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15822626#comment-15822626
]
Hadoop QA commented on SENTRY-1546:
---
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12847440/SENTRY-1546.002.patch
against master.
{color:green}Overall:{color} +1 all checks pass
{color:green}SUCCESS:{color} all tests passed
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/2253/console
This message is automatically generated.
> Generic Policy provides bad error messages for Sentry exceptions
>
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch
>
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response respose = requestHandle(new RequestHandler() {
> @Override
> public Response handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15752908#comment-15752908
]
Hadoop QA commented on SENTRY-1546:
---
Here are the results of testing the latest attachment
https://issues.apache.org/jira/secure/attachment/12843503/SENTRY-1546.001.patch
against master.
{color:red}Overall:{color} -1 due to an error
{color:red}ERROR:{color} failed to apply patch (exit code 1):
The patch does not appear to apply with p0, p1, or p2
Console output:
https://builds.apache.org/job/PreCommit-SENTRY-Build/2207/console
This message is automatically generated.
> Generic Policy provides bad error messages for Sentry exceptions
>
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Attachments: SENTRY-1546.001.patch
>
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response respose = requestHandle(new RequestHandler() {
> @Override
> public Response handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15752890#comment-15752890
]
kalyan kumar kalvagadda commented on SENTRY-1546:
-
Added code changes to SentryStore to have proper message (where the exceptions
are originated). If this is done we can avoid decorating them later.
I saw issue with exceptions created while role create/drop and made required
changes.
> Generic Policy provides bad error messages for Sentry exceptions
>
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response respose = requestHandle(new RequestHandler() {
> @Override
> public Response handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15749961#comment-15749961
]
Alexander Kolbasov commented on SENTRY-1546:
Note that the code and decorations are different dependent on the model.
> Generic Policy provides bad error messages for Sentry exceptions
>
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response respose = requestHandle(new RequestHandler() {
> @Override
> public Response handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15749560#comment-15749560
]
kalyan kumar kalvagadda commented on SENTRY-1546:
-
I see that exception is decorated with more information. I need to test to see
why is not applied.
> Generic Policy provides bad error messages for Sentry exceptions
>
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
>Affects Versions: 1.8.0
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response respose = requestHandle(new RequestHandler() {
> @Override
> public Response handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
