Skip to content

[PP-3653] Remove LCP non-EPUB deprioritize behavior and setting.#3032

Merged
dbernstein merged 3 commits intomainfrom
PP-3653-remove-deprioritize-lcp-feature
Feb 10, 2026
Merged

[PP-3653] Remove LCP non-EPUB deprioritize behavior and setting.#3032
dbernstein merged 3 commits intomainfrom
PP-3653-remove-deprioritize-lcp-feature

Conversation

@dbernstein
Copy link
Contributor

@dbernstein dbernstein commented Feb 9, 2026

Description

Remove LCP deprioritize setting (deprioritize_lcp_non_epubs)

  • Drop deprioritize_lcp_non_epubs from FormatPriorities and
    FormatPrioritiesSettings; remove artificial LCP non-EPUB sort logic.
  • Update OPDS API constructors (base, for_distributors, odl) to build
    FormatPriorities with only prioritized_drm_schemes and
    prioritized_content_types.
  • Remove related tests and the TestFormatPrioritiesSettings test class.
  • Add Alembic migration to delete the deprioritize_lcp_non_epubs key
    from integration_configurations.settings (JSONB); no schema change.

Motivation and Context

Now that both mobile apps support streaming LCP audiobook ZIP files, we can remove the temporary setting to de-prioritize LCP audiobooks (the actual label for the setting is “De-Prioritize LCP non-EPUBs”)

https://ebce-lyrasis.atlassian.net/browse/PP-3653

How Has This Been Tested?

Checklist

  • I have updated the documentation accordingly.
  • All new and existing tests passed.

@dbernstein dbernstein marked this pull request as draft February 9, 2026 22:17
@codecov
Copy link

codecov bot commented Feb 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.07%. Comparing base (ded9054) to head (714618a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3032      +/-   ##
==========================================
- Coverage   93.07%   93.07%   -0.01%     
==========================================
  Files         480      480              
  Lines       43677    43668       -9     
  Branches     6028     6026       -2     
==========================================
- Hits        40651    40642       -9     
  Misses       1960     1960              
  Partials     1066     1066              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dbernstein dbernstein marked this pull request as ready for review February 9, 2026 22:59
@dbernstein dbernstein requested a review from a team February 9, 2026 23:42
@jonathangreen jonathangreen added the DB migration This PR contains a DB migration label Feb 10, 2026
Copy link
Member

@jonathangreen jonathangreen left a comment

Choose a reason for hiding this comment

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

Looks good

Copy link
Member

@jonathangreen jonathangreen left a comment

Choose a reason for hiding this comment

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

Noticed a couple minor things after approving this one that would be nice to get sorted out before merging. Added a couple small comments.

Revises: 9c3f2b3fba1b
Create Date: 2026-02-09

"""
Copy link
Member

Choose a reason for hiding this comment

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

Was this generated with alembic revision? The filename doesn't have the alembic revision ID in it like the rest of our revisions do. Maybe regenerate it to get the correct metadata included?

Comment on lines +19 to +22
op.execute(
"UPDATE integration_configurations SET settings = settings - 'deprioritize_lcp_non_epubs' "
"WHERE settings ? 'deprioritize_lcp_non_epubs'"
)
Copy link
Member

Choose a reason for hiding this comment

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

Minor: Not strictly necessary, but we usually wrap these in sa.text to stop SA from emitting warnings. Might be good to wrap this for consistency.

Copy link
Member

@jonathangreen jonathangreen left a comment

Choose a reason for hiding this comment

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

I'm going to mark this one is "request changes" to make sure the migration issue gets sorted out before merging.

@@ -0,0 +1,26 @@
"""Remove deprioritize_lcp_non_epubs setting

Revision ID: a1b2c3d4e5f6
Copy link
Member

Choose a reason for hiding this comment

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

Looking at this revision ID, it totally wasn't generated by alembic revision. That looks like something generated by AI. You need to make sure if AI generates your migrations that it generates them via alembic revision instead of just faking it and making up a plausible looking migration file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I did request AI to use alembic for the revision but clearly I need to be more specific.

@dbernstein dbernstein force-pushed the PP-3653-remove-deprioritize-lcp-feature branch from c872b14 to 714618a Compare February 10, 2026 19:18
Copy link
Member

@jonathangreen jonathangreen left a comment

Choose a reason for hiding this comment

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

Looks good

@dbernstein dbernstein merged commit 7227bbc into main Feb 10, 2026
19 checks passed
@dbernstein dbernstein deleted the PP-3653-remove-deprioritize-lcp-feature branch February 10, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DB migration This PR contains a DB migration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants