-
Notifications
You must be signed in to change notification settings - Fork 17
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
PR-3244 S3 credential fix #644
Conversation
de662fd
to
f4a39ef
Compare
if policy is None: | ||
template_vars = { | ||
"contentstring": "You do not have permission to access any data.", | ||
"title": "Could not access data", |
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.
I don't think this really matters but it seems weird to have contentstring
before title
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.
Yea, unfortunately this is the order it's in everywhere. I'd love to refactor this whole file and fix all sorts of stuff like this, but last time I tried to do that I got blocked because people didn't like that refactoring would cause additional commits to be created in the history...
mock_make_html_response.assert_called_once_with( | ||
{ | ||
"contentstring": "You do not have permission to access any data.", | ||
"title": "Could not access data", |
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.
Again, a little weird
f4a39ef
to
becb60d
Compare
The main fix comes from updating the rain-api-core dependency to the version that will dedupe statement resources