-
Notifications
You must be signed in to change notification settings - Fork 81
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
samples: Add AuthorizedView examples #2192
Conversation
Change-Id: I77bb799eb448275928bcf0b562c0d4e06870c96d
System.out.println(greeting); | ||
} | ||
} catch (NotFoundException e) { | ||
System.err.println("Failed to write to non-existent authorized view: " + e.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe update this to "Failed to write to non-existent table or authorized view: " .. cause I think if the table doesn't exist, it would also be NotFound?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm but here we are targeting the authorized view only. In the AFE we would perform spanner lookups for the authorized view. So if the parent table doesn't exist, the NotFound error returned from the AFE is because of the missing authorized view instead of the missing table.
It's similar to when we are trying to write to a table and got a NotFound error, we wouldn't print the error msg as "Failed to write to non-existent table or instance"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, make sense :)
} | ||
} catch (NotFoundException e) { | ||
System.err.println("Failed to write to non-existent authorized view: " + e.getMessage()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we also need to catch permission denied? If the writer doesn't have write permission to the view? Same for other examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
The other two read-only examples wouldn't receive permission denied error for reads outside of the authorized view. The server would simply return empty rows.
Change-Id: Ic0e4a16dfd7e17b3580669e7a8d53a9b427c6049
/gcbrun |
Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot. |
/gcbrun |
Change-Id: I77bb799eb448275928bcf0b562c0d4e06870c96d
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> ☕️
If you write sample code, please follow the samples format.