Skip to content

Conversation

@Vagoasdf
Copy link
Contributor

@Vagoasdf Vagoasdf commented Jan 13, 2026

Ticket INTS-369

Description Of Changes

This PR enhances the polling status override system to support nuanced completion states. Previously, status overrides could only return a simple bool (complete or not), but some APIs have multiple completion states like:

  • Complete → polling done, fetch results
  • Complete_No_Records_Found → polling done, skip result fetch (no data for this user)
    The new PollingStatusResult schema allows status overrides to signal "complete but skip result request", avoiding unnecessary HTTP calls when there's no data to fetch.

Backward Compatibility: Existing status overrides returning bool continue to work unchanged.

Code Changes

New Schema

  • Added PollingStatusResult model with is_complete: bool and skip_result_request: bool = False
    ** Updated Factory**
  • Updated validate_polling_status_override_function to accept both bool and PollingStatusResult return types
    Updated Strategy
  • Added skip flow handling for both access (access_data=[]) and erasure (rows_masked=0) operations
  • _check_sub_request_status now returns PollingStatusResult (wraps legacy bool returns)
  • Extracted _process_single_sub_request method for cleaner code organization

Steps to Confirm

(Requires Fidesplus)
Backwards Compatibility

  1. Set up a Polling integration (I.E Bazaarvoice).
  2. Run an access request with any email
  3. Check that the status is passing correctly ( Should continue running)
    ** New functionality**
  4. Set up Highspot Integration from this PR
  5. Run an access request with any email
  6. Check that the status check gets Complete_No_Records_Found
  7. The request should skip

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link
Contributor

vercel bot commented Jan 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Review Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Jan 15, 2026 3:03pm
fides-privacy-center Ignored Ignored Jan 15, 2026 3:03pm

@Vagoasdf Vagoasdf changed the title Ints 369 highspot imrpove polling status Ints 369 highspot improve polling status Jan 14, 2026
@Vagoasdf Vagoasdf marked this pull request as ready for review January 15, 2026 15:03
@Vagoasdf Vagoasdf requested a review from a team as a code owner January 15, 2026 15:03
@Vagoasdf Vagoasdf requested review from erosselli and galvana and removed request for a team January 15, 2026 15:03
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 15, 2026

Greptile Summary

This PR enhances the polling status override system to support nuanced completion states. Previously, status overrides could only return a simple bool (complete or not), but some APIs have multiple completion states like "Complete" and "Complete_No_Records_Found". The new PollingStatusResult schema allows status overrides to signal "complete but skip result request", avoiding unnecessary HTTP calls when there's no data to fetch.

Key Changes:

  • Added PollingStatusResult schema with is_complete and skip_result_request fields
  • Updated _check_sub_request_status to wrap legacy bool returns for backward compatibility
  • Extracted _process_single_sub_request method for cleaner code organization
  • Added skip flow handling: sets access_data=[] for access operations and rows_masked=0 for erasure operations when skip_result_request=True
  • Updated factory validation to accept both bool and PollingStatusResult return types
  • Added comprehensive test coverage for both legacy and new return types

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The implementation demonstrates excellent backward compatibility by wrapping legacy bool returns in PollingStatusResult. The code changes are well-structured with proper extraction of the _process_single_sub_request method. Comprehensive test coverage validates both the new functionality and backward compatibility scenarios. The skip flow logic is straightforward and correctly handles both access and erasure operations.
  • No files require special attention

Important Files Changed

Filename Overview
src/fides/api/schemas/saas/async_polling_configuration.py Added PollingStatusResult schema to support nuanced completion states with skip_result_request flag
src/fides/api/service/async_dsr/strategies/async_dsr_strategy_polling.py Updated strategy to handle PollingStatusResult, extract _process_single_sub_request method, and support skip flow for both access and erasure
src/fides/api/service/saas_request/saas_request_override_factory.py Updated validation to accept both bool (legacy) and PollingStatusResult (new) return types for polling status overrides
tests/ops/service/async_dsr/polling/test_async_polling_strategy.py Added extensive tests for skip flow, backward compatibility with bool returns, and multi-sub-request scenarios

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 15, 2026

Greptile's behavior is changing!

From now on, if a review finishes with no comments, we will not post an additional "statistics" comment to confirm that our review found nothing to comment on. However, you can confirm that we reviewed your changes in the status check section.

This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR".

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.

2 participants