Skip to content

Conversation

tvandort
Copy link
Contributor

@tvandort tvandort commented Jul 9, 2025

Closes ENG-898

Description Of Changes

Retry for pulling custom-fides.css on failures.
Do not disable custom-fides.css polling on failures that are not 404.

Code Changes

  • list your code changes here

Steps to Confirm

404 Behavior shuts off polling & Retry does not occur

  1. Run the Privacy Center & Fides API.
  2. Ensure that there is no api/v1/custom-asset/custom-fides.css uploaded.
  3. Load https://localhost:3001/fides.js twice
  4. Ensure that api/v1/custom-asset/custom-fides.css is only called once by looking at the server logs.
  5. Ensure that Privacy Center does not re-attempt pulling CSS. Use FIDES_PRIVACY_CENTER__LOG_LEVEL=debug to see attempt messages.

Non-404 Behavior will retry and leave CSS polling on

  1. Run the Privacy Center
  2. Edit src/fidesplus/api/routes/custom_asset.py to return a 500
  3. Load https://localhost:3001/fides.js
  4. Ensure that api/v1/custom-asset/custom-fides.css is called multiple times via Fides API & Privacy Center logs
  5. Load https://localhost:3001/fides.js, observe the same things from step 4 indicating that CSS polling is enabled still

200 behavior is maintained

  1. Run the Privacy Center & Fides API
  2. Upload custom CSS to api/v1/custom-asset/custom-fides.css
  3. Load https://localhost:3001/fides.js twice
  4. Observe that api/v1/custom-asset/custom-fides.css is called once

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
  • 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

Copy link

vercel bot commented Jul 9, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
fides-plus-nightly ⬜️ Ignored (Inspect) Visit Preview Jul 10, 2025 9:05pm
fides-privacy-center ⬜️ Ignored (Inspect) Jul 10, 2025 9:05pm

@thabofletcher
Copy link
Contributor

thabofletcher commented Jul 10, 2025

Looks good! 🚀 Worked as best I could test it, thanks for the steps to help with that - I did most of my testing by just modifying the custom_asset.py in fideplus

@tvandort tvandort merged commit 575fa27 into main Jul 10, 2025
17 checks passed
@tvandort tvandort deleted the ENG-898 branch July 10, 2025 21:27
Copy link

cypress bot commented Jul 10, 2025

fides    Run #13101

Run Properties:  status check passed Passed #13101  •  git commit 575fa27621: ENG-898 (#6319)
Project fides
Branch Review main
Run status status check passed Passed #13101
Run duration 00m 50s
Commit git commit 575fa27621: ENG-898 (#6319)
Committer Tom Van Dort
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 5
View all changes introduced in this branch ↗︎

tvandort added a commit that referenced this pull request Jul 10, 2025
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