Skip to content

Conversation

@pkontek
Copy link

@pkontek pkontek commented Dec 18, 2025

This pull request updates the _optionset_map method to cache empty results instead of returning None when no items are found. This change ensures that empty lookups are cached, which can improve performance by avoiding repeated queries for missing option sets.

Caching improvements:

  • Modified _optionset_map in src/PowerPlatform/Dataverse/data/_odata.py to cache empty results in _picklist_label_cache and return an empty dictionary instead of None when no items are found.

Copilot AI review requested due to automatic review settings December 18, 2025 07:56
@pkontek pkontek requested a review from a team as a code owner December 18, 2025 07:56
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes the picklist label retrieval logic by caching empty results when no option sets are found, preventing redundant queries for non-existent data.

Key Changes:

  • Modified _optionset_map to cache empty results instead of returning None
  • Return empty dictionary {} instead of None for consistency

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +1018 to +1020
# cache empty result
self._picklist_label_cache[cache_key] = {"map": {}, "ts": now}
return {}
Copy link

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

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

Changing the return type from None to {} is a breaking change for consumers who check for None (e.g., if result is None). This could cause existing code to behave incorrectly. Consider deprecating the None return in a separate release or updating all callers to handle empty dict, or document this breaking change explicitly in release notes.

Copilot uses AI. Check for mistakes.
Copy link
Author

Choose a reason for hiding this comment

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

It’s the same value returned when the Attribute is not a picklist. In my opinion, this approach is more consistent.

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.

1 participant