Skip to content

remote/client: allow already locked resources #832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

Emantor
Copy link
Member

@Emantor Emantor commented Sep 15, 2021

Description

Allow a place to still be locked if one of the resources is acquired but
was previously acquired by the same place.

Signed-off-by: Rouven Czerwinski [email protected]

Not really sure how to test this.

Checklist

  • CHANGES.rst has been updated
  • PR has been tested

Fixes #748

Allow a place to still be locked if one of the resources is acquired but
was previously acquired by the same place.

Signed-off-by: Rouven Czerwinski <[email protected]>

Fixes labgrid-project#748
@Emantor Emantor added the fix label Sep 15, 2021
@Emantor Emantor requested a review from jluebbe September 15, 2021 14:33
@codecov
Copy link

codecov bot commented Sep 15, 2021

Codecov Report

Merging #832 (59d3108) into master (abef525) will decrease coverage by 0.0%.
The diff coverage is 50.0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master    #832     +/-   ##
========================================
- Coverage    56.8%   56.8%   -0.1%     
========================================
  Files         144     144             
  Lines       10672   10674      +2     
========================================
+ Hits         6068    6069      +1     
- Misses       4604    4605      +1     
Impacted Files Coverage Δ
labgrid/remote/client.py 44.7% <50.0%> (+<0.1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update abef525...59d3108. Read the comment docs.

@jluebbe
Copy link
Member

jluebbe commented Sep 15, 2021

We can't just ignore the inconsistent state here. In the situation reported in #748, this would only allow using this resource by the same place, but if the resource is now configured to match for a different place, it cannot be acquired.
I suspect that the issue already exists before the failing acquire. @egrigore: Do you see any other tracebacks on the exporter or coordinator correlated to the issue?

Copy link
Member

@jluebbe jluebbe left a comment

Choose a reason for hiding this comment

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

This should not be merged as is, see my comment above.

@Emantor
Copy link
Member Author

Emantor commented Sep 16, 2021

This should not be merged as is, see my comment above.

I agree, closing.

@Emantor Emantor closed this Sep 16, 2021
@egrigore
Copy link

@jluebbe sorry for the late response . We have seen this problem again today but there is nothing special related to it on exporter or coordinator journalctl . There aren't clear steps to isolate the problem and also it doesn't happen only on a specific board to relate it to that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Place cannot be acquired because one of its resources is acquired by itself
3 participants