Skip to content

[feat] Added file uploads endpoints, and add file uploads examples #270

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

tsinglinrain
Copy link

Overview

This PR adds support for file uploads endpoints to the notion-sdk-py, following the design and conventions of the official Notion API documentation and the notion-sdk-js. #268

Implementation Details

notion_client/api_endpoints.py
Following the existing coding style, I referred to the official documentation and the file uploads implementation in notion-sdk-js when designing the endpoint logic.
The request uses files to handle binary content and data for other metadata, keeping them clearly separated.

notion_client/client.py
I extended the existing request logic to support files and data as additional parameters. The code now dynamically constructs the request based on parameter type.

notion_client/helper.py
The official JavaScript SDK only supports single-chunk uploads and relies on external libraries for splitting large files. In Python, there is no audited or popular native implementation, so I wrote a custom split_file function.

split_file: synchronously splits files into smaller chunks.

split_file_async: provides similar functionality using asynchronous iteration.

These utility functions are designed to be reusable and thus included in helper.py.

notion_client/notion_mime_detector.py
Due to inconsistencies in Python’s built-in mimetypes across platforms (e.g., Windows 11 registry values override expected MIME types), I implemented a dedicated MIME type detector.
This module ensures platform-consistent behavior and explicitly supports the MIME types Notion accepts. It also includes a utility for listing supported file types.

examples/file_uploads
Includes usage examples to demonstrate the new file upload functionality.

Additional Thoughts

In the future, this feature could be further abstracted into a higher-level interface where the user only needs to provide the file path and name, and the SDK handles the rest (e.g., auto-selecting upload mode).

Final Note

  • I’ve been a long-time user of notion-sdk-py and truly appreciate the work done by the maintainers.

  • This is my first PR with a relatively large amount of code, and I’m still learning. There may be issues or non-idiomatic patterns in the implementation, and I warmly welcome any suggestions or corrections. Thank you for your time and guidance!

Copy link

codecov bot commented Jun 23, 2025

Codecov Report

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

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #270   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          304       328   +24     
=========================================
+ Hits           304       328   +24     

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

@ramnes
Copy link
Owner

ramnes commented Jun 24, 2025

Hey, thanks for your work!

Here's what I see from a quick first pass:

  • a few things need to be cleaned:
    • no need to rewrite split_files and other utils in the examples, let's just import them
    • let's remove some comments / commented code / prints (especially those in Chinese)
    • don't depend on external libraries that are not dependencies, such as requests
  • I think we should stay closer to the JS SDK and implement the formDataParams logic
  • tests must be added so that we keep at least 100% coverage.

What do you think?

@tsinglinrain
Copy link
Author

Thank you for your review and feedback. I highly agree with the points you raised.

In fact, my initial submission included some elements not directly related to the Notion SDK, such as split_files and mime handling, as you correctly pointed out. I will remove these from the client to align with the JavaScript SDK implementation. The PR will focus solely on the file upload feature to maintain the project's purity.

Regarding the examples, I will completely remove those extraneous parts, retaining only a minimal implementation for file uploads.

Concerning the send method in the file upload endpoints, I fully agree with your suggestion to "stay closer to the JS SDK and implement the formDataParams logic". After reconsideration, my approach will be as follows below (details).

For other cleanup items, I will clean up the redundant comments , and irrelevant dependencies.

About test coverage: I must admit I have no prior experience with this. I am actively learning and writing tests now to meet the project's high standards.

I will continue refining the code, which may take some time. I will update the PR immediately upon completion. Thank you again for your professional guidance!

file upload send
def send(self, file_upload_id: str, **kwargs: Any) -> SyncAsync[Any]:
    """Send a file upload

    *[🔗 Endpoint documentation](https://developers.notion.com/reference/send-a-file-upload)*
    """  # noqa: E501
    return self.parent.request(
        path=f"file_uploads/{file_upload_id}/send",
        method="POST",
        form_data=pick(kwargs, "file", "part_number"),
        auth=kwargs.get("auth"),
    )
    def _build_request(
        self,
        method: str,
        path: str,
        query: Optional[Dict[Any, Any]] = None,
        body: Optional[Dict[Any, Any]] = None,
        form_data: Optional[Dict[Any, Any]] = None,
        auth: Optional[str] = None,
    ) -> Request:
        ...

        if form_data is not None:
            files = {}
            data = {}

            for key, value in form_data.items():
                if key == "file":
                    files[key] = value
                else:
                    data[key] = value
            return self.client.build_request(
                method,
                path,
                params=query,
                json=body,
                files=files,
                data=data,
                headers=headers,
            )
        else:
            return self.client.build_request(
                method,
                path,
                params=query,
                json=body,
                headers=headers,
            )

- Updated `notion_client\api_endpoints.py` and `notion_client\client.py`: refined the file sending logic to align with `formDataParams` handling in `notion-sdk-js`
- Removed MIME type detection
- Replaced custom file splitting function with the existing `filesplit` library
- Removed numerous redundant comments and unrelated print statements
- Completed most of `tests\test_endpoints.py`
…lso `None` could lead to an error in `httpx` due to calling `.encode()` on a `None` object
- Updated VCR configuration in `conftest.py` to preserve key response headers such as `content-encoding` and `content-type`
- Formatted `api_endpoints.py` for code style consistency
- Added assertions for the `send` section in `test_endpoints.py`
… version

- Adjusted related code in `conftest.py` to ensure compatibility with the downgraded VCR version
… bytes

- Removed unused dependencies from the code under `example/file_uploads`
- Improved test coverage of `client.py` by adding handling for Gzip decompression, form data processing, JSON decode errors, non-API errors, and timeout exceptions
- Fixed code formatting issues
… decompression

- Further standardized code formatting
…les in parallel to optimize upload efficiency
@FlorianWilhelm
Copy link
Contributor

Hi @tsinglinrain, cool that you started to work on this feature. May I ask what the current status is?
When this PR is merged, I am hoping to implement the same feature in Ultimate Notion, which relies on notion-sdk-py. So the sooner you are ready, the sooner I can start working on a high-level implementation 😄

@tsinglinrain
Copy link
Author

Hi, @FlorianWilhelm, sorry for the delay. The last commit already completed all the intended work. I had originally planned to do another round of checks for any potential issues, but I got caught up with job hunting and the related preparations, and couldn't find the time.

I'll be taking two weeks off starting tomorrow, and during that time I’ll be fully dedicating myself to finishing up this PR.

Hi @tsinglinrain, cool that you started to work on this feature. May I ask what the current status is? When this PR is merged, I am hoping to implement the same feature in Ultimate Notion, which relies on notion-sdk-py. So the sooner you are ready, the sooner I can start working on a high-level implementation 😄

@tsinglinrain
Copy link
Author

Hi, when you have a moment, could you please review this PR? I believe the file upload feature is mostly done in the latest commit. There might still be some issues, and I plan to resolve them within the next two weeks.

Copy link
Owner

@ramnes ramnes left a comment

Choose a reason for hiding this comment

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

Thanks for the work! C.f. comments for the change requests.

I may be wrong but I feel like we should be able to simplify things related to gzip, the code here seems unnecessarily low-level. (Notion SDK codebase doesn't even contain "gzip"!)

@@ -8,10 +8,84 @@
from notion_client import AsyncClient, Client


# Fix for VCR compatibility issue with cassettes that have gzip content
Copy link
Owner

Choose a reason for hiding this comment

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

What issue? :)

Copy link
Author

Choose a reason for hiding this comment

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

While running the tests, I encountered an AttributeError: 'NoneType' object has no attribute 'encode'. To properly handle cases where content is None, I added this function.

This piece of code was written with the help of AI, as I’m not very familiar with testing. That’s why I didn’t move the third-party import statements to the top, but instead directly added the function, making changes while preserving as much of the original code as possible for easier review. I‘m sorry, and I’m afraid I may not be able to provide detailed explanations or make further modifications to this part.

…e" key to content-based matching, aligning with TS SDK approach

- remove: eliminate all complex gzip processing logic
- Updated `client.py` tests
- Increased `client.py` test coverage
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