Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace webpub manifest parser with pydantic (PP-1367) #2163

Open
wants to merge 2 commits into
base: feature/add-pydantic-opds-models
Choose a base branch
from

Conversation

jonathangreen
Copy link
Member

@jonathangreen jonathangreen commented Nov 13, 2024

Description

Replace our custom web manifest parser with Pydantic models.

I replace the validation we were doing via a patched json schema with the pydantic parser, since it gives better errors and more closely matches what we are expecting in Palace.

Edit: I broke this one into two PRs to make it a little easier to review. This PR actually implements the new models in our existing code, #2168 adds the models.

Motivation and Context

JIRA: PP-1367

Note: After this one goes in we should cut a new release of https://github.com/ThePalaceProject/webpub-manifest-parser and then mark it as archived, since we won't be using it anymore.

How Has This Been Tested?

  • Ran unit tests
  • Imported some feeds locally:
    • Test Palace Marketplace feed
    • Palace Bookshelf feed

Checklist

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

@jonathangreen jonathangreen force-pushed the feature/replace-webpub-manifest-parser branch from 6f7438a to 2ae2600 Compare November 13, 2024 16:40
Copy link

codecov bot commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 92.08633% with 11 lines in your changes missing coverage. Please review.

Project coverage is 90.91%. Comparing base (399768d) to head (da3e71a).

Files with missing lines Patch % Lines
src/palace/manager/core/opds2_import.py 90.12% 4 Missing and 4 partials ⚠️
src/palace/manager/util/__init__.py 71.42% 0 Missing and 2 partials ⚠️
src/palace/manager/api/odl/importer.py 96.55% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                         Coverage Diff                          @@
##           feature/add-pydantic-opds-models    #2163      +/-   ##
====================================================================
+ Coverage                             90.79%   90.91%   +0.12%     
====================================================================
  Files                                   359      359              
  Lines                                 41294    41171     -123     
  Branches                               8924     8885      -39     
====================================================================
- Hits                                  37492    37432      -60     
+ Misses                                 2481     2429      -52     
+ Partials                               1321     1310      -11     

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

@jonathangreen jonathangreen changed the title Replace webpub manifest parser with pydantic Replace webpub manifest parser with pydantic (PP-1367) Nov 13, 2024
@jonathangreen jonathangreen marked this pull request as ready for review November 13, 2024 18:04
@jonathangreen jonathangreen added the feature New feature label Nov 13, 2024
@jonathangreen jonathangreen requested a review from a team November 13, 2024 18:04
@jonathangreen jonathangreen force-pushed the feature/replace-webpub-manifest-parser branch from 4a9279d to 50fc11c Compare November 14, 2024 17:14
@jonathangreen jonathangreen changed the base branch from main to feature/add-pydantic-opds-models November 14, 2024 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant