Gevent refactoring #862
Gevent refactoring #862
Conversation
|
While i take a look |
|
@acepace take a look into the issues this fixes. |
monkey/infection_monkey/control.py
Outdated
| @@ -81,7 +81,7 @@ def find_server(default_tunnel=None): | |||
| if ControlClient.proxies: | |||
| debug_message += " through proxies: %s" % ControlClient.proxies | |||
| LOG.debug(debug_message) | |||
| requests.get("https://%s/api?action=is-up" % (server,), # noqa: DUO123 | |||
| requests.get(f"https://{server}/api?action=is-up", # noqa: DUO123 | |||
There was a problem hiding this comment.
What version of python are we baselined on? 3.6 or 3.7?
| @@ -5,7 +5,7 @@ | |||
|
|
|||
|
|
|||
| def get_windows_commands_to_schedule_jobs(): | |||
| return f'schtasks /Create /SC monthly /TN {SCHEDULED_TASK_NAME} /TR {SCHEDULED_TASK_COMMAND}' | |||
| return f'schtasks /Create /SC monthly /F /TN {SCHEDULED_TASK_NAME} /TR {SCHEDULED_TASK_COMMAND}' | |||
There was a problem hiding this comment.
Shouldn't be part of this PR but shrug
There was a problem hiding this comment.
Fixed, didn't notice other commits
monkey/monkey_island.py
Outdated
| from gevent import monkey | ||
| monkey.patch_all() |
There was a problem hiding this comment.
Confusing name, wouldn't it be nicer to
from gevent import monkey as gevent_monkey
since our entire codebase uses monkey a lot
| from monkey_island.cc.services.reporting.report_generation_synchronisation import safe_generate_reports, \ | ||
| is_report_being_generated |
There was a problem hiding this comment.
Shouldn't be part of this PR but shrug
| @@ -1,15 +1,15 @@ | |||
| import logging | |||
| import threading | |||
| from gevent.lock import Semaphore | |||
|
LGTM |
c87a43d to
a9af6fe
Compare
…aphore and other small style fixes
05c214c to
d0fda6b
Compare
Codecov Report
@@ Coverage Diff @@
## develop #862 +/- ##
===========================================
- Coverage 60.56% 60.52% -0.04%
===========================================
Files 166 165 -1
Lines 4953 4948 -5
===========================================
- Hits 3000 2995 -5
Misses 1953 1953
Continue to review full report at Codecov.
|
|
Can you rebase and we merge? |
There was a problem hiding this comment.
Overall looks good to me. If @acepace doesn't have any further comments I'm good with it.
My only concern is the transition from single-thread to multi-thread can cause unexpected errors in different parts in code which weren't designed properly or didn't take this into consideration.
I suggest running some/all automatic blackbox tests on this branch to make sure we didn't break anything
|
@itaymmguardicore so let's do a proper tests before the next release and catch all errors, not only something caused by gevent. Cost of running in depth tests > info that specific bug happened due to gevent. |
What does this PR do?
Fixes #858 #857
Refactored tornado WSGI container into gevent WSGI container.
PR Checklist
Testing Checklist