Filter regions based on account availabilities in get_regions function #625
Conversation
zliang-akamai
requested review from
jriddle-linode and
lgarber-akamai
and removed request for
a team
zliang-akamai
added
the
testing
|
Should be bypassed if the token doesn't have account API access. Converting to draft to work on it. |
lgarber-akamai
left a comment
lgarber-akamai
left a comment
There was a problem hiding this comment.
LGTM other than the comment above, great work!
jriddle-linode
left a comment
jriddle-linode
left a comment
There was a problem hiding this comment.
Works locally, looks good!
There was a problem hiding this comment.
Pull request overview
This PR enhances the get_regions function to filter regions based on account-level availability constraints, helping to reduce intermittent errors during integration tests by ensuring selected regions have the necessary capabilities enabled for the account.
Key Changes:
- Added account availability checking to filter regions based on account-specific capabilities
- Added error handling with logging for cases where account availability retrieval fails
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| and set(capabilities) | ||
| .intersection(ALL_ACCOUNT_AVAILABILITIES) | ||
| .issubset( | ||
| account_regional_availabilities.get( | ||
| v.id, | ||
| ( | ||
| [] | ||
| if account_regional_availabilities | ||
| else ALL_ACCOUNT_AVAILABILITIES | ||
| ), | ||
| ) | ||
| ) |
There was a problem hiding this comment.
The nested conditional logic with multi-line set operations is difficult to read and understand. Consider extracting this logic into a separate helper function (e.g., is_region_available_for_capabilities) that takes the region, capabilities, and account_regional_availabilities as parameters. This would improve readability and make the code easier to test and maintain.
| and set(capabilities) | ||
| .intersection(ALL_ACCOUNT_AVAILABILITIES) |
There was a problem hiding this comment.
The set intersection set(capabilities).intersection(ALL_ACCOUNT_AVAILABILITIES) is computed for every region in the list comprehension. Consider calculating this once before the list comprehension and storing it in a variable to avoid redundant computation.
3 participants
📝 Description
This may be helpful to reduce intermittent errors during integration tests.
✔️ How to Test
Run any test you like, for example: