Skip to content

Conversation

nyoma-diamond
Copy link
Contributor

A quick-and-dirty implementation of the lazy datadir that uses a shared data directory mimicking shared_datadir.

This is very useful for when there are a lot of data files and organizing them for every single test / test file instead of using a single shared data directory is very cumbersome, but copying all of the files each time shared_datadir is used induces lots of excessive copying/file usage.

@nicoddemus
Copy link
Collaborator

Seems reasonable, thanks @nyoma-diamond!

Could you please provide tests for the new fixture?

@nyoma-diamond
Copy link
Contributor Author

Yup sure thing :)

@nyoma-diamond
Copy link
Contributor Author

nyoma-diamond commented Jul 29, 2025

@nicoddemus Hopefully those tests are sufficient?

It's worth noting that I've made an explicit design choice (admittedly somewhat arbitrarily, but I think makes sense) to have lazy_shared_datadir create tmp_path / "data" upon initialization. Alternatively it may be acceptable to not create the directory explicitly and just let it be created the first time a file is requested, but I think creating it explicitly ensures minimal confusion over the behavior / order of operations, the overhead from empty directories is very small, and there should never be any situations where the user doesn't utilize the created directory (if the user doesn't want the directory, they don't need lazy_shared_datadir to begin with).

That said, if you disagree with the design choice, it's an easy change.

@gabrielcnr
Copy link
Owner

I think the presented new behaviour for lazy_shared_datadir is consistent with the existing behaviour of shared_datadir by choosing the tmp_path / 'data' as directory to be created. The creation of the directory is inexpensive, and being a temporary directory shouldn't be a big deal in terms of leaving residuals behind (even if the copies never happen). I think this is good enough. I will let @nicoddemus share his thoughts too.

Copy link
Collaborator

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

LGTM!

I will merge/release tomorrow to give others a chance to review. 👍

@nicoddemus nicoddemus merged commit ce17181 into gabrielcnr:master Jul 30, 2025
20 checks passed
@nicoddemus
Copy link
Collaborator

1.8.0 released, thanks again @nyoma-diamond for the PR!

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