Skip to content

Test basic operation with all backends - #9594

Merged
pllim merged 1 commit into
astropy:masterfrom
aarchiba:cache-backend
Nov 12, 2019
Merged

Test basic operation with all backends#9594
pllim merged 1 commit into
astropy:masterfrom
aarchiba:cache-backend

Conversation

@aarchiba

@aarchiba aarchiba commented Nov 11, 2019 •
edited by pllim
Loading

Copy link
Copy Markdown
Contributor

Description

The cache mechanism depends on python's shelve module, which can use one of three backends (dumb, gdbm and ndbm). These sometimes have different failure modes, so create a test that tries all backends available to a given interpreter.

The hope was that this would trigger a warning (cache problems at the end of clear_download_cache()) so that I could be sure that I had banished it, but in the event that doesn't seem to happen on Linux. Nevertheless I think I fixed that problem.

Relevant to #7928

EDIT: Follow up of #9182

@astropy-bot astropy-bot Bot added the utils label Nov 11, 2019
@bsipocz bsipocz added this to the v4.0 milestone Nov 11, 2019
@pllim

pllim commented Nov 11, 2019

Copy link
Copy Markdown
Member

I installed this patch and applied the conftest.py and setup.cfg settings on top of it, then I ran the tests locally with python setup.py test -P utils --remote-data, but unfortunately there is one failure, as attached:

test_log.txt

@aarchiba

aarchiba commented Nov 12, 2019

Copy link
Copy Markdown
Contributor Author

Okay, I was able to trigger the warning - it happens with dbm.dumb when a previously nonexistent cache is cleared. Fixed now. (Actually all I needed wase ENOENT instead of EEXIST but now I'm able to trigger the behaviour.)

pllim
pllim reviewed Nov 12, 2019
View reviewed changes
Comment thread astropy/utils/tests/test_data.py Resolved
pllim
pllim reviewed Nov 12, 2019
View reviewed changes

@pllim pllim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some minor stylistic comments below. I'll take this for a spin and report back. Thanks!

Comment thread astropy/utils/data.py Resolved
Comment thread astropy/utils/data.py Resolved
Comment thread astropy/utils/tests/test_data.py Resolved
Comment thread astropy/utils/tests/test_data.py Resolved
Comment thread astropy/utils/tests/test_data.py Resolved
@pllim

pllim commented Nov 12, 2019 •
edited
Loading

Copy link
Copy Markdown
Member

Update: Warning is gone on my machine with this patch! 🎉

Remote data failure is unrelated (#9600 ).

@pllim pllim mentioned this pull request Nov 12, 2019
Merged
@aarchiba

aarchiba commented Nov 12, 2019

Copy link
Copy Markdown
Contributor Author

Sorry about the debugging detritus.

pllim
pllim approved these changes Nov 12, 2019
View reviewed changes

@pllim pllim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you very much, @aarchiba !

@pllim
pllim merged commit 8f0d5c0 into astropy:master Nov 12, 2019
@aarchiba

aarchiba commented Nov 12, 2019

Copy link
Copy Markdown
Contributor Author

Glad I could help! And it's good to have some tests that explore all the backends.

@pllim pllim mentioned this pull request Nov 14, 2019
Closed
bsipocz pushed a commit that referenced this pull request Nov 18, 2019
@pllim @bsipocz
Merge pull request #9594 from aarchiba/cache-backend
9d26f8c 
Test basic operation with all backends
@pllim pllim mentioned this pull request Nov 18, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pllim pllim pllim approved these changes

Assignees

No one assigned

Labels

no-changelog-entry-needed utils

Projects

None yet

Milestone

v4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@aarchiba @pllim @bsipocz