Skip to content

Schedule jobs bugfix#861

Merged
VakarisZ merged 3 commits intoguardicore:developfrom
VakarisZ:schedule_jobs_bugfix
Dec 8, 2020
Merged

Schedule jobs bugfix#861
VakarisZ merged 3 commits intoguardicore:developfrom
VakarisZ:schedule_jobs_bugfix

Conversation

@VakarisZ
Copy link
Contributor

@VakarisZ VakarisZ commented Oct 12, 2020

What does this PR do?

Fixed 2 issues:

  • Scheduled job gets created, but never deleted
  • If scheduled job is already present when monkey wants to create it, schtasks prompts a question and monkey hangs indefinitelly

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

  • Added relevant unit tests?
  • Have you successfully tested your changes locally? Elaborate:

    Tested what happens if monkey job is already present
    Tested if job is created and deleted afterwards
@codecov
Copy link

codecov bot commented Oct 12, 2020 •
edited
Loading

Codecov Report

Merging #861 into develop will increase coverage by 0.00%.
The diff coverage is n/a.

Impacted file tree graph

@@ Coverage Diff @@ ## develop #861 +/- ## ======================================== Coverage 60.52% 60.53% ======================================== Files 165 165 Lines 4948 4949 +1 ======================================== + Hits 2995 2996 +1  Misses 1953 1953
Impacted Files Coverage Δ
common/utils/exceptions.py 100.00% <0.00%> (ø)

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 53f3625...a777431. Read the comment docs.
@VakarisZ VakarisZ force-pushed the schedule_jobs_bugfix branch from 5356ab4 to 7abafb7 Compare October 12, 2020 13:47
itaymmguardicore
itaymmguardicore suggested changes Oct 13, 2020
View reviewed changes
monkey/monkey_island/cc/services/version_update.py Outdated
reply = requests.get(url, timeout=7)
except requests.exceptions.RequestException:
logger.info("Can't get latest monkey version, probably no connection to the internet.")
raise NoInternetError
Copy link
Contributor

@itaymmguardicore itaymmguardicore Oct 13, 2020

Choose a reason for hiding this comment

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

This might result from lack of internet connection, but also if our update server fails to respond for some reason.
I would change the exception to something like UpdateServerUnreachableException

I think the term "update server" better describes the server than "version server", but it's more important to stay consistent. So either change the rest of the variable names here to "update server" or call the exception VersionServerUnreachableException
acepace
acepace reviewed Oct 19, 2020
View reviewed changes
monkey/common/utils/exceptions.py
""" Raise to indicate the reason why registration is not required """


class VersionServerConnectionError(Exception):
Copy link
Contributor

@acepace acepace Oct 19, 2020

Choose a reason for hiding this comment

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

Should be part of this PR?
Copy link
Contributor Author

@VakarisZ VakarisZ Nov 10, 2020

Choose a reason for hiding this comment

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

Probably not, but since this is a small refactoring on separate commits, I think it's fine
@VakarisZ VakarisZ merged commit ac71a3e into guardicore:develop Dec 8, 2020
@VakarisZ VakarisZ deleted the schedule_jobs_bugfix branch December 8, 2020 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants

@VakarisZ @acepace @itaymmguardicore @shreyamalviya