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

Add optional limit argument to get_block #294

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

Conversation

c0j0s
Copy link
Contributor

@c0j0s c0j0s commented Mar 5, 2021

#292

although setting a new fixed limit works, adding an optional limit argument would be better if notion decide to change the query limit in the future.

@dorileo
Copy link

dorileo commented Mar 9, 2021

Well, overall the PR looks good. +1

@@ -360,6 +360,7 @@ def __init__(
sort=[],
calendar_by="",
group_by="",
limit=100

Choose a reason for hiding this comment

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

Is it possible to increase the default limit for CollectionQuery? The current value doesn't fetch all rows from my database with ~200 items. I didn't run into any API errors with a higher value (100000) during my short test.

Choose a reason for hiding this comment

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

I agree, I tried same value for limit and it runs without an issue (https://charts.mathix.ninja/).

But it would be nice to have some sort of pagination, as after 100000 documents the same issue will occur.

Copy link

Choose a reason for hiding this comment

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

Have you tried: limit=-1? How is it working with 10000+ ?

mathix420 added a commit to mathix420/notion-charts that referenced this pull request Mar 9, 2021
Copy link
Contributor Author

@c0j0s c0j0s left a comment

Choose a reason for hiding this comment

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

@felixII You can supply a limit argument like this cv.collection.get_rows(limit=200) to fetch all your rows.

I know that this method isnt applicable to everyone as they have varying table sizes, therefore I have added support to fetch the total size of the collection first before requesting for the actual data.

Now you can do this cv.collection.get_rows(limit=-1), backend it will query by your total collection size instead of a fixed limit.

This isnt an elegant solution though as it doubles the api calls in the background, and also I'm not able to verify it for large table with 10K over rows 😅.

@xiantang
Copy link

@jamalex plz take a look!

@accounts01
Copy link

accounts01 commented Mar 21, 2021

this is a pretty paralyzing issue. does anyone know how long it will take for it to get merged in?

@gnestor
Copy link

gnestor commented Mar 22, 2021

this is a pretty paralyzing issue. does anyone know how long it will take for it to get merged in?

In the meantime, you can pip install git+https://github.com/jamalex/notion-py.git@refs/pull/294/merge or replace notion-py with git+https://github.com/jamalex/notion-py.git@refs/pull/294/merge in your requirements.txt. I have tested and this fixed my issues 👍

@accounts01
Copy link

thanks a lot @gnestor! that's exactly what i needed :) and it's so much cleaner than fiddling with the code in site-packages which is what i ended up doing 🙈

@younho9
Copy link

younho9 commented Mar 27, 2021

Thanks a lot @gnestor! pip install git+https://github.com/jamalex/notion-py.git@refs/pull/294/merge command installs specific versions of git repo on notion-py well.

However, I have problem using setuptools to specify the merge version of the git repo as dependency in my python package.

Referring to the this link, the install_requires specified as follows, but this does not work properly.

setuptools.setup(
    ...    
    install_requires=[
        "notion @ git+https://github.com/jamalex/notion-py.git@refs/pull/294/merge"
    ],
    ...
)

Could you help me with this problem by any chance?

@gnestor
Copy link

gnestor commented Mar 27, 2021

My setuptools chops are rusty, but try replacing notion @ git+https://github.com/jamalex/notion-py.git@refs/pull/294/merge with git+https://github.com/jamalex/notion-py.git@refs/pull/294/merge

@younho9
Copy link

younho9 commented Mar 28, 2021

I tried the following setup.py settings, but the following error occurs.

setuptools.setup(
    ...    
    install_requires=[
        "git+https://github.com/jamalex/notion-py.git@refs/pull/294/merge"
    ],
    ...
)
error in narkdown setup command: 'install_requires' must be a string or list of strings containing valid project/version requirement specifiers; Parse error at "'+https:/'": Expected stringEnd

Also, I have tried the following with this answer.

But this method has a problem when uploading to pypi.

HTTPError: 400 Bad Request from https://upload.pypi.org/legacy/
Invalid value for requires_dist. Error: Can't have direct dependency: 'notion @ git+https://github.com/jamalex/notion-py.git@refs/pull/294/merge'

I need to find a way to set git repo to direct dependence when uploading to pypi.

@gnestor
Copy link

gnestor commented Mar 28, 2021

It looks like your original approach was the right way to do it in setuptools but I'm not sure how you can point to a git ref...

@younho9
Copy link

younho9 commented Mar 29, 2021

I also couldn't find a way to upload a package that have a git repo as a dependency to pypi.

Instead, I solved the problem by importing the git forked version directly.

Thank you for your reply.

@vzts
Copy link

vzts commented May 13, 2021

Hi @jamalex Can it be merged?

@miletoda
Copy link

Have you find the way to load/read more than 100 records at a time?

@es-arif
Copy link

es-arif commented Sep 2, 2021

Please audit and merge if possible. This is affecting many users!

This is stretching out to StackOverFlow

@paulaceccon
Copy link

Thanks a lot @gnestor! pip install git+https://github.com/jamalex/notion-py.git@refs/pull/294/merge command installs specific versions of git repo on notion-py well.

However, I have problem using setuptools to specify the merge version of the git repo as dependency in my python package.

Referring to the this link, the install_requires specified as follows, but this does not work properly.

setuptools.setup(
    ...    
    install_requires=[
        "notion @ git+https://github.com/jamalex/notion-py.git@refs/pull/294/merge"
    ],
    ...
)

Could you help me with this problem by any chance?

Use the commit:

notion@git+https://github.com/jamalex/notion-py.git@fe8ac41e1c10a229c040bd7cadd9429a66fbcda3

@andrewspode
Copy link

Is there a reason this hasn't been merged in yet? It's breaking lots of things :)

@fanuch
Copy link

fanuch commented May 31, 2022

FWIW I have started moving all my code across to the official Notion API.
May not have feature parity but it will get there.

Sick of sourcing on a PR for a fix.

@tyePhDCandy
Copy link

I run the code to install commit with pip, but setup.py error occurred.

I run the following code, with hash code of this commit.

# fe8ac41e1c10a229c040bd7cadd9429a66fbcda3 is the hash code of this commit.
pip install git+https://github.com/jamalex/notion-py.git@fe8ac41e1c10a229c040bd7cadd9429a66fbcda3

I get an error related to setup.py.

Collecting git+https://github.com/jamalex/notion-py.git@fe8ac41e1c10a229c040bd7cadd9429a66fbcda3
  Cloning https://github.com/jamalex/notion-py.git (to revision fe8ac41e1c10a229c040bd7cadd9429a66fbcda3) to c:\users\uotik\appdata\local\temp\pip-req-build-g11n7s9k
  Running command git clone --filter=blob:none --quiet https://github.com/jamalex/notion-py.git 'C:\Users\uotik\AppData\Local\Temp\pip-req-build-g11n7s9k'
  Running command git rev-parse -q --verify 'sha^fe8ac41e1c10a229c040bd7cadd9429a66fbcda3'
  Running command git fetch -q https://github.com/jamalex/notion-py.git fe8ac41e1c10a229c040bd7cadd9429a66fbcda3
  Running command git checkout -q fe8ac41e1c10a229c040bd7cadd9429a66fbcda3
  Resolved https://github.com/jamalex/notion-py.git to commit fe8ac41e1c10a229c040bd7cadd9429a66fbcda3
  Preparing metadata (setup.py) ... error
  error: subprocess-exited-with-error

  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [6 lines of output]
      Traceback (most recent call last):
        File "<string>", line 2, in <module>
        File "<pip-setuptools-caller>", line 34, in <module>
        File "C:\Users\uotik\AppData\Local\Temp\pip-req-build-g11n7s9k\setup.py", line 4, in <module>
          long_description = fh.read()
      UnicodeDecodeError: 'gbk' codec can't decode byte 0x92 in position 11951: illegal multibyte sequence
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
hint: See above for details.

@c0j0s c0j0s closed this May 20, 2024
@emmby
Copy link

emmby commented Jul 1, 2024

@c0j0s or @jamalex Can you re-open this PR? Many of us are linking directly to the PR in our pip requirements.txt, and my deployments are now failing now that the PR is closed.

Context: https://stackoverflow.com/a/55959742/82156

@c0j0s
Copy link
Contributor Author

c0j0s commented Jul 1, 2024

@c0j0s or @jamalex Can you re-open this PR? Many of us are linking directly to the PR in our pip requirements.txt, and my deployments are now failing now that the PR is closed.

Context: https://stackoverflow.com/a/55959742/82156

I closed this PR as it was included in #352 and was merged into the master branch.

I'll reopen it again, didn't realise closing this would have such side effect.

@c0j0s c0j0s reopened this Jul 1, 2024
@emmby
Copy link

emmby commented Jul 1, 2024

Even better, I didn't realize the changes had been merged in elsewhere. I'll switch to head, ty!

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.