Skip to content

Python: Remove duplicate pop in InMemoryCacheProvider.remove#5795

Merged
eavanvalkenburg merged 1 commit into
microsoft:mainfrom
taisirhassan:purview-cache-remove-duplicate-pop
May 19, 2026
Merged

Python: Remove duplicate pop in InMemoryCacheProvider.remove#5795
eavanvalkenburg merged 1 commit into
microsoft:mainfrom
taisirhassan:purview-cache-remove-duplicate-pop

Conversation

@taisirhassan

Copy link
Copy Markdown
Contributor

Motivation and Context

The remove method in InMemoryCacheProvider (purview package) contains a duplicate self._cache.pop(key, None)
call. The second call is dead code: the first pop has already either removed the key or returned None, and there
is no await between the two statements that could allow another coroutine to re-add the key. The redundant line is
misleading to readers (suggests it's guarding against something it isn't) and adds noise to the diff history.

Description

Removes the second self._cache.pop(key, None) call in InMemoryCacheProvider.remove.

Behavior is identical before and after for every input:

  • If the key was present, the first pop removes it and decrements _current_size_bytes. The second pop (now
    removed) found nothing.
  • If the key was absent, both pop calls were no-ops.

No public API change. No test changes required — existing test_cache.py tests continue to pass.

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the Contribution
    Guidelines
  • All unit tests pass, and I have added new tests where possible
  • [] Is this a breaking change? If yes, add "[BREAKING]" prefix to the title of the PR.

The second self._cache.pop(key, None) call is a guaranteed no-op: the first pop has already removed the key (or returned None), and there is no await between the two statements that could allow another coroutine to re-add it. Removing the dead line clarifies intent without changing behavior.
Copilot AI review requested due to automatic review settings May 13, 2026 01:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 removes a redundant second pop call in InMemoryCacheProvider.remove within the Purview Python package. The change eliminates dead code while preserving identical runtime behavior and keeping the public API unchanged.

Changes:

  • Remove a duplicate self._cache.pop(key, None) in InMemoryCacheProvider.remove, leaving the size-accounting logic intact.

@moonbox3

Copy link
Copy Markdown
Contributor

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
packages/purview/agent_framework_purview
   _cache.py690100% 
TOTAL33872390688% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
6675 30 💤 0 ❌ 0 🔥 1m 38s ⏱️

@moonbox3 moonbox3 added this pull request to the merge queue May 19, 2026
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks May 19, 2026
@eavanvalkenburg eavanvalkenburg added this pull request to the merge queue May 19, 2026
Merged via the queue into microsoft:main with commit 3f522a8 May 19, 2026
42 of 43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants