Skip to content

Conversation

tsinglinrain
Copy link
Contributor

  1. Added data source endpoints and refactored database endpoints

    1. For the DataSourcesEndpoint class, the official documentation is inconsistent in casing, using both Data Source and data source. I chose to follow the official documentation exactly as written.
    2. For example, see the first line of Retrieve a data source and Query a data source.
    3. In the DataSourcesEndpoint class, the order of Body Params follows the official JS SDK rather than the API reference documentation.
  2. Added support for is_locked in page update.

  3. Added additional_data in error responses.

(Note: This PR mainly focuses on data source and database updates, and does not fully cover all changes introduced in the latest TS version.)

  1. Test coverage improvements:
    1. Since the existing tests\cassettes used an old notion-version, I replaced them in bulk. However, the new version tests failed because decode_compressed_response: True caused VCR to attempt decompression, expecting a body.string format, while the new format uses content. Setting it to False resolved the issue.
    2. My assumption: vcrpy 5.1.0 + httpx 0.28.1 produces an unstable format mix, while vcrpy 6.0.2 + httpx 0.28.1 consistently outputs the body.string format. Therefore, I upgraded vcrpy to version 6.0.2 (keeping httpx 0.28.1), and removed the previously written compatibility function. With this change, httpx automatically handles compression, so I set decode_compressed_response: False, ensuring cassette files retain their raw state — making debugging easier and reducing potential VCR-related bugs.
    3. As a result, all tests\cassettes were regenerated, except for tests\test_helpers.py.
    4. The expression re.sub(RE_PAGE_ID, r1.uri, "") was likely incorrect. In re.sub(pattern, repl, string), the second parameter (repl) should be the replacement text, not the input string. I fixed this with a small change to improve robustness and error handling.

related:#275

- Refactored `DatabasesEndpoint` methods (note: no pytest tests included)
…`notion-sdk-js`: in `DataSourcesEndpoint.update`, added `"parent"` to the `body` parameter
…538` from `notion-sdk-js`: added support for `is_locked` in update methods of page and database APIs
- Updated `tests\cassettes` to change `notion-version: - '2025-09-03'`
- Modified `tests\conftest.py` for format compatibility
- Added `"is_locked"` to the `body` of the `update` function in `DatabasesEndpoint`
Copy link

codecov bot commented Sep 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (c86d608) to head (de4ff70).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #286   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          367       375    +8     
=========================================
+ Hits           367       375    +8     

☔ 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.

@tsinglinrain
Copy link
Contributor Author

Could I Remove support for Python 3.7?

- Improved tests for `request_id` and `additional_data`
- Enhanced `_parse_response` to handle `additional_data` and `request_id`
- Improved initialization tests for `APIResponseError`
@ramnes
Copy link
Owner

ramnes commented Sep 19, 2025

Hey @tsinglinrain, thanks for your work here!

From a (very) quick look, it seems to go in the right direction, but please try to keep the PR as thin as possible so that it's easier to review. I'd rather review multiple small PRs than one big PR.

For example, I don't think I'd approve adding more debug logs. But on the other side, you removed _patch_vcr_from_serialized_response and explained why, which is basically an answer to issue #274, and so I would merge that straight away.

For Python 3.7, I'm most likely okay to remove it, but why do we want that?

Also, please remove Chinese comments. :)

@tsinglinrain
Copy link
Contributor Author

tsinglinrain commented Sep 20, 2025

@ramnes Thank you for your feedback.

Sorry about this, but please don’t review it just yet — I plan to simplify this PR and keep only the data source part, and then you can take another look afterward. (I intend to split it into two PRs.)

I merged some changes along the way because they felt convenient to include, but I realize now that I didn’t fully understand the importance of keeping branches focused — apologies for that.

The debug logging changes were added because I noticed in TypeScript we have this:

this.log(LogLevel.INFO, "request success", {
  method,
  path,
  ...("request_id" in responseJson && responseJson.request_id
    ? { requestId: responseJson.request_id }
    : {}),
})

So I went ahead and added debug logs for this part as well.

Since vcrpy has been upgraded to 6.0.2, which no longer supports Python 3.7, I’d also like to ask if it would be possible to drop Python 3.7 support. (To make the tests pass, I updated github/workflows/test.yml and set
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"], removing Python 3.7.)

@ramnes
Copy link
Owner

ramnes commented Sep 23, 2025

Sorry about this, but please don’t review it just yet — I plan to simplify this PR and keep only the data source part, and then you can take another look afterward. (I intend to split it into two PRs.)

I merged some changes along the way because they felt convenient to include, but I realize now that I didn’t fully understand the importance of keeping branches focused — apologies for that.

No worries, looking forward!

The debug logging changes were added because I noticed in TypeScript we have this:

Oh, interesting. Happy to review a PR that makes logging in this library identical to notion-sdk-js logging.

Since vcrpy has been upgraded to 6.0.2, which no longer supports Python 3.7, I’d also like to ask if it would be possible to drop Python 3.7 support. (To make the tests pass, I updated github/workflows/test.yml and set
python-version: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13"], removing Python 3.7.)

Makes sense! You can remove it.

@sashanclrp
Copy link

@ramnes what is missing here to get this merged & published in the package?
Sorry I'm new to contributing to OSS projects and want to understand the dynamics so I don't go through the embarrasment i went through before

- Removed mentions of Python 3.7 support from `README.md` and other docs
- Updated minimum supported Python version to 3.8
@tsinglinrain
Copy link
Contributor Author

@ramnes I have simplified this PR to keep only the data source and database parts. Could you please review it?

@tsinglinrain
Copy link
Contributor Author

addition_data and request_id are only supported in the new version (2025-09-03). It’s a bit tricky, so I plan to open another PR on top of this one after it gets merged.

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.

3 participants