Basic pagination support for result sets.

[jheddings]

Creates an iterator from a database query, which fetches new results as needed.

If this looks like a reasonable approach, I will look into adding support for the other endpoints.

[aahnik]

Everything looks right! This is really helpful.

Have you run pre-commit run?

The docstrings don't comply with pydocstyle. Little modifications will fix all the warnings.

[jheddings]

Have you run pre-commit run?

How do I run this manually?

[aahnik]

How do I run this manually?

By running the command pre-commit run -a

For more details read the contributing guidelines page in the website.

https://ramnes.github.io/notion-sdk-py/contributing/contributing/

[jheddings]

Okay, added the additional paging endpoints and fixed the pre-commit failures.

[jheddings]

Hang on... Found a bug.

[aahnik]

this looks nice 🎉

[aahnik]

Hi @jheddings, it would be great if you please add an example to use this new features.

[ramnes]

Can you find a way to make this generic? The ResultSetIterator children classes feel noisy to me, they're basically all the same. A simple way I see to achieve this would be to give the endpoint method rather than the client to the iterator constructor.

[ramnes]

And yes, as @aahnik said, this needs an example somewhere at the very least, and ideally tests. :)

[jheddings]

Added examples and some documentation... I added a general form of the iterator, but personally I like having the explicit iterators for clarity in the code. It also keeps doors open in the future if there is any need to handle results differently, for example if we use this in the upcoming ORM feature. We can remove if you guys disagree.

As for testing, I'll keep looking into it. It's difficult without mocked endpoints, but I'll see what I can do.

[ramnes]

Yeah I'd prefer to keep the code as concise as possible and have only one way to do things, please. :)

[ramnes]

For the tests, I don't think you actually need to call the API nor to mock it, you can just give any function that returns what you expect to receive from an endpoint method.

[jheddings]

Yeah I'd prefer to keep the code as concise as possible and have only one way to do things, please. :)

It's your world, boss ;) Removed the specific iterator classes and updated docs / examples.

For the tests, I don't think you actually need to call the API nor to mock it, you can just give any function that returns what you expect to receive from an endpoint method.

I added a basic test with a mock endpoint. I'm not sure if there is a place to submit the tests for automatic execution, but it is working locally :)

[nehiljain]

Just a thought, may be you can combine all the iterators in 1 file. It feels fairly repetitive.

[jheddings]

It is... But the point of the examples are to provide specific, easy to read use cases. Combining all endpoints in a single file makes it harder to pick out the point of each example. I'm happy with the way it is, but of course someone is always free to make further changes.

[aahnik]

Tests failed due to codecov upload error codecov/codecov-action#330

[jheddings]

It looks like the Codecov checks are missing the actions API upload token...

[aahnik]

codecov does not require token for public repos.

but they are recently having an issue. #54 fixes it.

@ramnes please add the codecov token in github secrets.

[codecov-commenter]

Codecov Report

Merging #51 (9b61dca) into main (612dea4) will decrease coverage by 11.15%.
The diff coverage is 95.08%.

❗ Current head 9b61dca differs from pull request most recent head 558dff0. Consider uploading reports for the commit 558dff0 to get more accurate results

@@             Coverage Diff             @@
##             main      #51       +/-   ##
===========================================
- Coverage   90.10%   78.94%   -11.16%     
===========================================
  Files           7        7               
  Lines         192      266       +74     
===========================================
+ Hits          173      210       +37     
- Misses         19       56       +37     

Impacted Files	Coverage Δ
notion_client/helpers.py	94.80% <95.08%> (+1.05%)	⬆️
notion_client/client.py	68.35% <0.00%> (-29.12%)	⬇️
notion_client/errors.py	73.21% <0.00%> (-19.47%)	⬇️
notion_client/typing.py	100.00% <0.00%> (ø)
notion_client/api_endpoints.py	69.23% <0.00%> (+0.93%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 612dea4...558dff0. Read the comment docs.

[ramnes]

I just ran checks again and it passed. Looks like Codecov guys fixed it. :)

[jheddings]

Any concerns merging this PR? It's starting to drift from main, so I've pulled the latest into the branch.

[ramnes]

Hey, sorry for the silence; I'm not sure about the API and it's on my to-do list to wrap my head around this but I haven't managed to do it yet.

[jheddings]

Hey, sorry for the silence; I'm not sure about the API and it's on my to-do list to wrap my head around this but I haven't managed to do it yet.

No worries... I have created a "higher level" module for the API to avoid cluttering up the core SDK. Check out notional if you are interested.

[ramnes]

You put a great effort there! Let's close this in favor of notional... for now. :P

[patrickwalton]

It'd be great to see this reopened and merged.

[ramnes]

@patrickwalton Basic pagination support has been implemented a few months ago, and that's shown in the README: https://github.com/ramnes/notion-sdk-py#utility-functions