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

adjusted fixed query limits #293

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

julrach
Copy link

@julrach julrach commented Mar 5, 2021

#292

Future thoughts: is there going to be pagination if the number of blocks surpasses 1000?

@hellvesper
Copy link

I think 100 is maximum allowed, because even 101 throws an error.

@julrach julrach closed this Mar 5, 2021
@julrach
Copy link
Author

julrach commented Mar 5, 2021

@hellvesper You right, IDK why it worked for a second for me. I think they have some incremental thing going on. Will do some more research.

@julrach julrach changed the title decreased 10000 fixed limits to 999 adjusted fixed query limits Mar 5, 2021
@julrach julrach reopened this Mar 5, 2021
@julrach
Copy link
Author

julrach commented Mar 5, 2021

@hellvesper Feel free to check this out again, I did some proper experimenting this time.

@ttran
Copy link

ttran commented Mar 5, 2021

from tzlocal import get_localzone

import notion
def call_load_page_chunk(self, page_id):

    if self._client.in_transaction():
        self._pages_to_refresh.append(page_id)
        return

    data = {
        "pageId": page_id,
        "limit": 100,
        "cursor": {"stack": []},
        "chunkNumber": 0,
        "verticalColumns": False,
    }

    recordmap = self._client.post("loadPageChunk", data).json()["recordMap"]

    self.store_recordmap(recordmap)

def call_query_collection(
    self,
    collection_id,
    collection_view_id,
    search="",
    type="table",
    aggregate=[],
    aggregations=[],
    filter={},
    sort=[],
    calendar_by="",
    group_by="",
):

    assert not (
        aggregate and aggregations
    ), "Use only one of `aggregate` or `aggregations` (old vs new format)"

    # convert singletons into lists if needed
    if isinstance(aggregate, dict):
        aggregate = [aggregate]
    if isinstance(sort, dict):
        sort = [sort]

    data = {
        "collectionId": collection_id,
        "collectionViewId": collection_view_id,
        "loader": {
            "limit": 1000000,
            "loadContentCover": True,
            "searchQuery": search,
            "userLocale": "en",
            "userTimeZone": str(get_localzone()),
            "type": type,
        },
        "query": {
            "aggregate": aggregate,
            "aggregations": aggregations,
            "filter": filter,
            "sort": sort,
        },
    }

    response = self._client.post("queryCollection", data).json()

    self.store_recordmap(response["recordMap"])

    return response["result"]

def search_pages_with_parent(self, parent_id, search=""):
    data = {
        "query": search,
        "parentId": parent_id,
        "limit": 100,
        "spaceId": self.current_space.id,
    }
    response = self.post("searchPagesWithParent", data).json()
    self._store.store_recordmap(response["recordMap"])
    return response["results"]

notion.store.RecordStore.call_load_page_chunk = call_load_page_chunk
notion.store.RecordStore.call_query_collection = call_query_collection
notion.client.NotionClient.search_pages_with_parent = search_pages_with_parent

The very hot, hot-fix based on this PR

@@ -310,7 +310,7 @@ def search_pages_with_parent(self, parent_id, search=""):
data = {
"query": search,
"parentId": parent_id,
"limit": 10000,
"limit": 100,
Copy link

Choose a reason for hiding this comment

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

I think it'd be better to store this as a constant somewhere. So if it changes later, there's only 1 place to update. Thoughts?

Copy link

@flaneur2020 flaneur2020 Aug 7, 2021

Choose a reason for hiding this comment

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

maybe it could be an argument in NotionClient.__init__() ?

@dorileo
Copy link

dorileo commented Mar 9, 2021

I believe #294 has a better approach.

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.

7 participants