Skip to content

PDFCLOUD-5554 Add client methods to watermark documents#19

Merged
datalogics-kam merged 12 commits intopdfrest:mainfrom
datalogics-cgreen:pdfcloud-5464-watermark
Feb 19, 2026
Merged

PDFCLOUD-5554 Add client methods to watermark documents#19
datalogics-kam merged 12 commits intopdfrest:mainfrom
datalogics-cgreen:pdfcloud-5464-watermark

Conversation

@datalogics-cgreen
Copy link
Copy Markdown
Contributor

@datalogics-cgreen datalogics-cgreen commented Jan 20, 2026

PDFCLOUD-5554

Adds a new Watermark PDF client helper and supporting payload/type definitions.

Introduces comprehensive unit and live test coverage for the watermark endpoint.

@datalogics-gwalczak datalogics-gwalczak changed the title Pdfcloud 5464 watermark PDFCLOUD-5464 watermark Jan 21, 2026
@datalogics-cgreen datalogics-cgreen changed the title PDFCLOUD-5464 watermark PDFCLOUD-5464 Add client methods to watermark documents Jan 30, 2026
@datalogics-cgreen datalogics-cgreen changed the title PDFCLOUD-5464 Add client methods to watermark documents PDFCLOUD-5554 Add client methods to watermark documents Jan 30, 2026
Comment thread src/pdfrest/models/_internal.py
Comment thread src/pdfrest/client.py Outdated
@datalogics-cgreen
Copy link
Copy Markdown
Contributor Author

@datalogics-kam If you like how text and image watermarks have been split up, I can squash the earlier version (commits).

@datalogics-cgreen datalogics-cgreen marked this pull request as ready for review February 12, 2026 22:11
@datalogics-kam
Copy link
Copy Markdown
Contributor

Let's avoid squashing for now, it makes it hard to reason about the changes. I think for human-originated stuff and small changes it's ok, but let's keep the coding agent history.

Copy link
Copy Markdown
Contributor

@datalogics-kam datalogics-kam left a comment

Choose a reason for hiding this comment

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

  1. Please remove default-merging validation; if anything, that kind of stuff belongs on the payload. If there are problems with default values going to the server, let's talk.
  2. Let's give the user one color with two possible types, and the default is RGB black.
  3. Codex complained about changing the name of PdfCmykColor, but I'm glad we fixed that inconsistency.

Codex findings from PR Review skill:

  1. [P2] Live coverage gap for split watermark endpoints (missing async image path)

    • watermark_pdf_with_image has live sync coverage but no matching live async success case, so transport parity is incomplete for the new split API surface.
    • Evidence: /Users/kam/src/dl/pdfrest-python/tests/live/test_live_watermark_pdf.py:76 covers sync image; no corresponding async image success test exists in that module.
    • Risk: async-only regressions in image watermark request/response behavior could ship undetected.
  2. [P2] Missing request-customization/timeout coverage for image watermark helper

    • The suite verifies extra_query/extra_headers/extra_body/timeout only for watermark_pdf_with_text, not for watermark_pdf_with_image.
    • Evidence: customization tests are text-only at /Users/kam/src/dl/pdfrest-python/tests/test_watermark_pdf.py:149 and /Users/kam/src/dl/pdfrest-python/tests/test_watermark_pdf.py:435; no image equivalent present.
    • Risk: plumbing regressions specific to the image helper (headers/query/body merge or timeout propagation) may slip through.

Comment thread src/pdfrest/client.py Outdated
Comment thread src/pdfrest/client.py Outdated
Comment thread src/pdfrest/client.py Outdated
Comment thread src/pdfrest/types/public.py
Comment thread src/pdfrest/models/_internal.py
Comment thread src/pdfrest/types/public.py
@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 18, 2026

Deploy Preview for pdfrest-python ready!

Name Link
🔨 Latest commit 7a26a78
🔍 Latest deploy log https://app.netlify.com/projects/pdfrest-python/deploys/69972e5149efb30008866d61
😎 Deploy Preview https://deploy-preview-19--pdfrest-python.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link
Copy Markdown
Contributor

@datalogics-kam datalogics-kam left a comment

Choose a reason for hiding this comment

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

Please address the changes requested. I will make modifications to AGENTS.md to cause Codex to follow the conventions in the future.

Comment thread src/pdfrest/models/_internal.py Outdated
Comment thread src/pdfrest/models/_internal.py Outdated
Comment thread src/pdfrest/models/_internal.py
@datalogics-cgreen
Copy link
Copy Markdown
Contributor Author

datalogics-cgreen commented Feb 19, 2026

01e398d adds a BeforeValidator intended to set the correct color parameter on the payload.

45471b1 and 2284c55 clean up further by setting the default text color on the client side and ensuring it's formatted properly in the request payload, respectively.

Copy link
Copy Markdown
Contributor

@datalogics-kam datalogics-kam left a comment

Choose a reason for hiding this comment

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

The text_color distribution to pdfRest fields is even more clever than I thought...approved.

@datalogics-kam datalogics-kam merged commit bad764a into pdfrest:main Feb 19, 2026
19 checks passed
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