Skip to content

Conversation

@hahaschool
Copy link
Contributor

@hahaschool hahaschool commented Jun 28, 2023

Fixes #84 . All tests passed locally.

@TomDonoghue
Copy link
Member

Thanks for the submission @hahaschool! Can you merge main in - it needs the updates to set which test versions to run

@hahaschool
Copy link
Contributor Author

Hi @TomDonoghue ,

Rebased, all tests passed locally.

Thanks.

@lisc-tools lisc-tools deleted a comment from codecov-commenter Sep 13, 2023
@TomDonoghue
Copy link
Member

Hey @hahaschool - sorry for the delay!

When returning a large number articles, you should really be using history (usehistory=True), as this stores the set of article IDs on the remote server, and then collects them in groups. This should in general avoid any issues with hitting URL limits, and is the general suggestion to collecting large numbers of articles.

That said, I think this still seems like a totally reasonable addition, and avoids any potential failures to collect a large number of articles when not using history. I did a quick check and direct refactor, but otherwise, I think we might as well add this - so I'm going to merge it in now!

@TomDonoghue TomDonoghue merged commit d8d32be into lisc-tools:main Sep 13, 2023
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.

URL length limit reached when fetching many articles

2 participants