Skip to content

Dealing with different shelve backends - #65

Open
Jackomatrus wants to merge 5 commits into
PokeAPI:masterfrom
Jackomatrus:master
Open

Jackomatrus wants to merge 5 commits into
PokeAPI:masterfrom
Jackomatrus:master

Conversation

@Jackomatrus

@Jackomatrus Jackomatrus commented Oct 2, 2026 •

Copy link
Copy Markdown

Tests were failing on my machine due to shelve creating three different cache files (api.cache.bak, api.cache.dat, and api.cache.dir). Only using os.remove() on api.cache might not work depending on the backend that the shelve module uses.

Shelve does not offer a native way to detect which files it created, so a glob pattern finds them all

@Naramsim

Naramsim commented Oct 6, 2026

Copy link
Copy Markdown
Member

Great addition, thanks a lot. We can merge

@Naramsim

Naramsim commented Oct 6, 2026

Copy link
Copy Markdown
Member

Should we document the new function somewhere?

@Jackomatrus

Copy link
Copy Markdown
Author

This is my first contribution to a public project, so I am not sure if (or where) we should.
It is of course true that other contributors might want to delete the cache for other reasons than testing.

@Naramsim

Naramsim commented Oct 9, 2026

Copy link
Copy Markdown
Member

Maybe @Jackomatrus we can rename the function you added to something like empty_cache() or something similar. Then you can one in the readme how to use it.

@Jackomatrus

Copy link
Copy Markdown
Author

I renamed the function to delete_cache (emptying the cache would suggest the cache to still be there but without data - the function actually deletes the files however).

I also made the function set CACHE_DIR, API_CACHE, and SPRITE_CACHE to None as otherwise they would point to non-existing paths after running delete_cache.
Let me know if this is okay to do here.

I added an example to the readme.

@Naramsim

Naramsim commented Oct 9, 2026

Copy link
Copy Markdown
Member

I ran the pipeline and the test are failing, could you take a look?

After deleting the cache would be there a way to recreate it?

@Jackomatrus

Copy link
Copy Markdown
Author

Sorry for missing that, I missed a functionality of shelve where shelve.open(path) creates the necessary files even if they were deleted.
Thus, your proposed name empty_cache is actually better fitting as cache.load() still works even after deleting the files.
Readme now also reflects the correct name cache.empty_cache

Tests now passed on my machine.

@Jackomatrus

Copy link
Copy Markdown
Author

Can you try again? I accidentally joined paths without checking for OS. I tested on both linux and windows now

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