Skip to content
This repository has been archived by the owner on Oct 31, 2023. It is now read-only.

Disabled virtualization check on Linux platforms #3949

Merged
merged 4 commits into from
Mar 6, 2019

Conversation

kmazurek
Copy link
Contributor

@kmazurek kmazurek commented Mar 4, 2019

Resolves: #3937

This change skips the virtualization check on Linux systems, always returning positive support status.

This change skips the virtualization check on Linux systems, always
returning positive support status.
@kmazurek kmazurek self-assigned this Mar 4, 2019
@kmazurek kmazurek requested a review from shadeofblue March 4, 2019 16:56
@ghost ghost added the in progress label Mar 4, 2019
if is_windows():
if is_linux():
# Virtualization support is not required to run Golem on Linux.
return True
Copy link
Contributor

Choose a reason for hiding this comment

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

Virtualization is not enabled per se, it's simply not required. I'd treat this fix as temporary. Maybe this call should be simply skipped by the UI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, hence the comment I left there. Another option would be to change this RPC / create a new one specific to the use case (i.e. checking for Docker support).

@kmazurek kmazurek requested a review from etam March 5, 2019 10:15
Copy link
Contributor

@etam etam left a comment

Choose a reason for hiding this comment

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

I agree with mfranciszkiewicz. This change makes a weird exception to the simple question "is virtualization enabled?". Instead of complicating the answer, we should fix GUI which asks the wrong question.

@kmazurek
Copy link
Contributor Author

kmazurek commented Mar 5, 2019

@shadeofblue: what's your take on this?

@shadeofblue
Copy link
Contributor

sigh... okay @zakaprov please change the name of the function to is_virtualization_satisfied or is_virtualization_requirement_satisfied ...

Copy link
Contributor

@etam etam left a comment

Choose a reason for hiding this comment

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

ok

@codecov
Copy link

codecov bot commented Mar 6, 2019

Codecov Report

Merging #3949 into develop will increase coverage by 0.03%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #3949      +/-   ##
===========================================
+ Coverage    87.77%   87.81%   +0.03%     
===========================================
  Files          216      216              
  Lines        18977    18979       +2     
===========================================
+ Hits         16658    16666       +8     
+ Misses        2319     2313       -6

@kmazurek kmazurek merged commit 7db8b63 into develop Mar 6, 2019
@kmazurek kmazurek deleted the disable-virtualization-check-linux branch March 6, 2019 14:13
@ghost ghost removed the in progress label Mar 6, 2019
shadeofblue added a commit that referenced this pull request Mar 20, 2019
…zation

Cherry pick of: Disabled virtualization check on Linux (#3949)
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants