Skip to content

Conversation

@kbuma
Copy link
Collaborator

@kbuma kbuma commented Dec 19, 2025

Summary

Major changes:

  • remove parallelization of server requests
  • re-implement handling of list criteria for parameters in endpoints that do not accept lists (this was tied into the parallelization code previously)
  • re-implement handling of list criteria that is too large for a single request (this was tied into the parallelization code previously)

Copy link
Collaborator

@tsmathis tsmathis left a comment

Choose a reason for hiding this comment

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

Not really much to say on my end, I am be curious though about the performance/execution time of this implementation vs. the parallel approach.

@kbuma
Copy link
Collaborator Author

kbuma commented Feb 6, 2026

@esoteric-ephemera @tsmathis I've addressed all outstanding comments. Ready for re-review.

tsmathis

This comment was marked as resolved.

@tsmathis
Copy link
Collaborator

tsmathis commented Feb 7, 2026

Nothing else to add from my end

@kbuma
Copy link
Collaborator Author

kbuma commented Feb 7, 2026

still a couple of .extend(...)s

this one should be fine, it's just the final step right? https://github.com/kbuma/api/blob/858accf6b00d90055eb21d642e0ac4eabf8ba921/mp_api/client/core/client.py#L732

this one is in a while loop: https://github.com/kbuma/api/blob/858accf6b00d90055eb21d642e0ac4eabf8ba921/mp_api/client/core/client.py#L806

@tsmathis addressed with fc25c1e

@esoteric-ephemera
Copy link
Collaborator

Thanks @kbuma - added missing settings back in for this to work. Looks like this current code block only triggers on a full download from S3

@esoteric-ephemera
Copy link
Collaborator

Also just a note: No further changes needed on my side, will merge this after our next deployment to isolate issues we're seeing

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.

3 participants