refactor(mounting): remove obsolete list-access-check CLI flag and logic#4722
refactor(mounting): remove obsolete list-access-check CLI flag and logic#4722meet2mky wants to merge 1 commit into
Conversation
…lag during bucket mounting
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4722 +/- ##
===========================================
+ Coverage 0 83.64% +83.64%
===========================================
Files 0 165 +165
Lines 0 20643 +20643
===========================================
+ Hits 0 17266 +17266
- Misses 0 2731 +2731
- Partials 0 646 +646
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request performs a cleanup of the codebase by removing the 'disable-list-access-check' flag and its corresponding implementation. The change simplifies the bucket setup process during mounting by relying on the StorageLayout API for bucket validation, rendering the previous list-based access check obsolete. Corresponding tests and configuration files have been updated to maintain consistency. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request removes the DisableListAccessCheck configuration and the associated ListObjects API check during the mount operation, transitioning the access check logic to rely on GetStorageLayout. The changes include the removal of the relevant struct fields, CLI flags, and configuration parameters across the codebase, along with corresponding test updates. Review feedback identifies potential nil pointer dereferences in the updated unit tests where bucket.Syncer is accessed after a failed SetUpBucket call; it is recommended to assert that the bucket itself is nil instead.
|
Hi @raj-prince, @abhishek10004, @vadlakondaswetha, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
2 similar comments
|
Hi @raj-prince, @abhishek10004, @vadlakondaswetha, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
|
Hi @raj-prince, @abhishek10004, @vadlakondaswetha, your feedback is needed to move this pull request forward. This automated reminder was triggered because there has been no activity for over 24 hours. Please provide your input when you have a moment. Thank you! |
Description
Cleanup the old code for disable list access check and flag. Flag usage is zero for the last 30 days in GKE: https://screenshot.googleplex.com/AueqegeHAJMeQZF
Link to the issue in case of a bug fix.
b/471129209
Testing details
Any backward incompatible change? If so, please explain.
NONE