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

fix: poll interval to request Notion #9

Merged
merged 5 commits into from
Jun 4, 2024

Conversation

telekosmos
Copy link
Contributor

In order to know when the export is finished, the poll interval to Notion was set to 50ms. Notion didn't bother this one (I guess they didn't pay attention ever before) but from little time ago, on this very frequent request a 429 is being received. So, as the export use to take from seconds to minutes, was tested than a value of 5000ms is ok and don't bother Notion (for now...).

In order to know when the export is finished, the poll interval to Notion
was set to 50ms. Notion didn't bother this one (I guess they didn't pay
attention ever before) but from little time ago, on this very frequent
request a 429 is being received. So, as the export use to take from
seconds to minutes, was tested than a value of 5000ms is ok and
don't bother Notion (for now...).
@yannbolliger
Copy link
Owner

Thanks for the PR. I have also seen the 429 behaviour recently.
However, I feel like 5000 is quite a big increase. Could we set the default to 100 and make the value configurable via the Config (see here)?

@telekosmos
Copy link
Contributor Author

Thanks for the PR. I have also seen the 429 behaviour recently. However, I feel like 5000 is quite a big increase. Could we set the default to 100 and make the value configurable via the Config (see here)?

I already tried to set it to 500 ms and didn't work (see here).

You're right regarding to the Config. I'll amend it

Add a property (optional) in the config to set the poll interval
to check if the export has finished (default to 2000ms).
Copy link
Owner

@yannbolliger yannbolliger left a comment

Choose a reason for hiding this comment

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

Let's acutally remove the function parameter now. Otherwise 👍

src/NotionExporter.ts Outdated Show resolved Hide resolved
Removed as parameter function and make it mandatory in config.
Copy link
Owner

@yannbolliger yannbolliger left a comment

Choose a reason for hiding this comment

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

Nice, thanks for contributing.

src/config.ts Outdated Show resolved Hide resolved
Set poll internal as an optional value in config and added private
constant with a default interval value in the case the config one
is not provided. Be aware it is used _only_ in a private method
src/NotionExporter.ts Outdated Show resolved Hide resolved
src/NotionExporter.ts Outdated Show resolved Hide resolved
@yannbolliger yannbolliger merged commit 5e0ae46 into yannbolliger:master Jun 4, 2024
3 checks passed
@yannbolliger
Copy link
Owner

Thx for contributing! Just released this: https://github.com/yannbolliger/notion-exporter/releases/tag/v0.6.2

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.

2 participants