Skip to content

Conversation

rjohanek
Copy link
Contributor

Jira ticket: https://broadworkbench.atlassian.net/browse/DC-1790

Addresses

User getting not authorized error despite being authorized. The issue was we were burying an error message that gave helpful information. The billing account on the dataset is disabled. Instead of catching that error and never showing it to the user, we will add more information to it and show the original error message.

Summary of changes

Show original error message and add additional information to it to help the user.
The original code buried an error message, this surfaces it.

Testing Strategy

This method was originally untested, so this adds unit tests for the method.

@rjohanek rjohanek requested a review from a team as a code owner October 20, 2025 15:30
@rjohanek rjohanek requested review from calypsomatic and samanehsan and removed request for a team October 20, 2025 15:30
permissions =
storageAsPet.testIamPermissions(
bucket, List.of(GCS_SOURCE_BUCKET_REQUIRED_PERMISSION), options);
logger.info("Permissions test result: {}", permissions);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to log the result of every bucket permissions check, since the important/useful bit is whether it fails. What do you think?

Copy link
Contributor

@samanehsan samanehsan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a small comment about logging, otherwise lgtm :octocat:

@rjohanek rjohanek enabled auto-merge (squash) October 21, 2025 16:14
Copy link

@rjohanek rjohanek merged commit 4247f88 into develop Oct 21, 2025
18 checks passed
@rjohanek rjohanek deleted the rj/dc-1790-not-authorized-on-ingest branch October 21, 2025 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants