Skip to content

fix: parse new JSONL bulk format, add missig fields & variants, rate limiting - #67

Open
voidcontext wants to merge 2 commits into
mendess:masterfrom
voidcontext:fix-bulk
Open

fix: parse new JSONL bulk format, add missig fields & variants, rate limiting#67
voidcontext wants to merge 2 commits into
mendess:masterfrom
voidcontext:fix-bulk

Conversation

@voidcontext

Copy link
Copy Markdown
Contributor

@voidcontext

Copy link
Copy Markdown
Contributor Author

@mendess There are a lot of test failures because of Scryfalls's rate limiting. I could just sprinkle a bit of "thread sleep" over the snippets in the doc comments, but that would be extremely ugly. Do you have any better suggestions?

@voidcontext

voidcontext commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@mendess There are a lot of test failures because of Scryfalls's rate limiting. I could just sprinkle a bit of "thread sleep" over the snippets in the doc comments, but that would be extremely ugly. Do you have any better suggestions?

I think I pushed a reasonable solution. Respecting rate limits by default is a good idea probably, and through feature flags it's possible to disable it if needed. This doesn't solve all the rate limiting related test issues, so I'll need to keep digging.

What do you think about this solution? Would it be acceptable?

Edit: obviously this would add an extra 100-500 ms delay to every requests even if it's not required. I'll try to come up with an improved solution.

Comment thread src/uri.rs
Comment thread src/uri.rs
@voidcontext
voidcontext force-pushed the fix-bulk branch 2 times, most recently from c82e2f4 to 2c3d576 Compare August 14, 2026 08:05
This commit is fixing some doctests as well, where the change in the
underlying Scryfall dataset made test ambiguous.
This change introduces request throttling, confgurable through rust
features at compile time.

The test script has been modified to resptect the rate limits.
@voidcontext

Copy link
Copy Markdown
Contributor Author

The build is green, all the known issues have been fixed (there's one flaky test though, that failed locally from time to time: search::tests::random_works_with_search_options)

I've made some changes to the test script that helped me during development (e.g. to build all the different feature permutations with scripts/test-features.sh build).

From a developer experience perspective (for contributors of this library), I am not really happy with the end result, because a simple cargo test will run into rate limit issues, but making the blocking feature default would mean that the library users would need to disable the default features to not be needlessly blocked in normal user scenarios. Maybe this should be documented in CONTRIBUTING.md.

@voidcontext
voidcontext marked this pull request as ready for review August 15, 2026 10:47
@voidcontext voidcontext changed the title fix: parse new JSONL bulk data format + add missig fields and variants fix: parse new JSONL bulk format, add missig fields & variants, rate limiting Aug 15, 2026
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.

1 participant