Skip to content

Restore upstream lock safety, avoid calling methods directly on upstream DB in test#13441

Merged
tldahlgren merged 2 commits intospack:developfrom
scheibelp:bugfix/upstream-db-test-locking
Oct 25, 2019
Merged

Restore upstream lock safety, avoid calling methods directly on upstream DB in test#13441
tldahlgren merged 2 commits intospack:developfrom
scheibelp:bugfix/upstream-db-test-locking

Conversation

@scheibelp
Copy link
Copy Markdown
Member

@scheibelp scheibelp commented Oct 25, 2019

Supersedes #13364

A merge created a semantic error for managing upstream DB locks: upstream DBs are not supposed to be locked at all but the ForbiddenLock object used to manage this was overwritten.

Additionally, an upstream db test was calling methods on upstream DBs that should only be called on the local DB (which knows how when to lock and when not to). This updates the test to avoid that, and also adds a regression test to make sure that writing to upstream DBs leads to an exception.

…nly be called on the local DB (which knows how when to lock and when not to)
@scheibelp scheibelp changed the title [WIP] Test bugfix: avoid calling methods directly on upstream DB [WIP] Restore upstream lock safety, avoid calling methods directly on upstream DB in test Oct 25, 2019
@scheibelp scheibelp changed the title [WIP] Restore upstream lock safety, avoid calling methods directly on upstream DB in test Restore upstream lock safety, avoid calling methods directly on upstream DB in test Oct 25, 2019
@scheibelp scheibelp requested a review from tldahlgren October 25, 2019 22:03
Copy link
Copy Markdown
Contributor

@tldahlgren tldahlgren left a comment

Choose a reason for hiding this comment

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

Reviewed the tests in context plus they both pass for me.

@tldahlgren tldahlgren mentioned this pull request Oct 25, 2019
1 task
@tldahlgren tldahlgren merged commit cfbac14 into spack:develop Oct 25, 2019
@tldahlgren
Copy link
Copy Markdown
Contributor

Thanks

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