Skip to content
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

Bugfix/grant app s3 access #1342

Merged
merged 4 commits into from
Oct 1, 2024
Merged

Conversation

michaeljcollinsuk
Copy link
Contributor

@michaeljcollinsuk michaeljcollinsuk commented Oct 1, 2024

📝 Summary

This PR relates to ministryofjustice/analytical-platform#5511

The bugs are described in the above ticket.

To resolve the S3 bucket name bug, I have fixed the regex that was used to ensure that it matches against the entire string.

To resolve the S3 app access bug, I have made sure that when an existing S3 bucket is linked to an app, that the current user arg is passed to the related task as expected.

🔍 What should the reviewer concentrate on?

  • Bug fixes

🧑‍💻 How should the reviewer test these changes?

  • Follow steps in the bug ticket linked above
  • Unit tests will run successfully

📚 Documentation status

  • No changes to the documentation are required
  • This PR includes all relevant documentation
  • Documentation will be added in the future because ... (see issue #...)

Copy link
Contributor

@jamesstottmoj jamesstottmoj left a comment

Choose a reason for hiding this comment

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

Ran changes locally. All working. LGTM

@michaeljcollinsuk michaeljcollinsuk merged commit 377f516 into main Oct 1, 2024
13 checks passed
@michaeljcollinsuk michaeljcollinsuk deleted the bugfix/grant-app-s3-access branch October 1, 2024 15:54
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.

🐞 Celery task errors raising high number of Sentry alerts
2 participants