Skip to content
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

Added request timeouts #885

Merged
merged 3 commits into from
Nov 16, 2020
Merged

Added request timeouts #885

merged 3 commits into from
Nov 16, 2020

Conversation

VakarisZ
Copy link
Contributor

@VakarisZ VakarisZ commented Nov 11, 2020

What does this PR do?

Fixes requests not having timeouts, which causes a delay in monkey executions or make it hang.
Also removed unused imports in one of the fingerprinters

Add any further explanations here.

PR Checklist

  • Have you added an explanation of what your changes do and why you'd like to include them?
  • Is the TravisCI build passing?

Testing Checklist

  • Have you successfully tested your changes locally? Elaborate:

    Tested by running sanity test

@VakarisZ VakarisZ changed the title Timeouts Added request timeouts Nov 11, 2020
@@ -9,6 +9,9 @@

import infection_monkey.monkeyfs as monkeyfs
import infection_monkey.tunnel as tunnel
from common.common_consts.timeouts import (LONG_REQUEST_TIMEOUT,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why ()?

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 think this is default isort style

Comment on lines 5 to 11
import logging
import subprocess

import nmb.NetBIOS
from impacket.dcerpc.v5 import epm, nrpc, transport

import infection_monkey.config
from infection_monkey.network.HostFinger import HostFinger
from infection_monkey.utils.environment import is_windows_os

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this part of the same commit/PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making new PR's for small cleanups like this would be annoying. It's on a separate, descriptive commit though.

Copy link
Contributor

@acepace acepace left a comment

Choose a reason for hiding this comment

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

Nits, LGTM

@VakarisZ VakarisZ merged commit 5ba1bf1 into guardicore:develop Nov 16, 2020
@VakarisZ VakarisZ deleted the timeouts branch February 18, 2022 06:43
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