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

Task API computation flow #4561

Merged
merged 1 commit into from
Aug 12, 2019
Merged

Task API computation flow #4561

merged 1 commit into from
Aug 12, 2019

Conversation

Krigpl
Copy link
Contributor

@Krigpl Krigpl commented Jul 31, 2019

Yet another two if conditions.

@Krigpl Krigpl force-pushed the task_api_compute branch from 3696169 to a24248b Compare July 31, 2019 14:02
@Krigpl Krigpl changed the base branch from new_taskcomputer to develop July 31, 2019 14:03
@Krigpl Krigpl force-pushed the task_api_compute branch from a24248b to a5bdfe0 Compare July 31, 2019 14:03
@Krigpl Krigpl force-pushed the task_api_compute branch from a5bdfe0 to 6c907e0 Compare July 31, 2019 14:47
Copy link
Contributor

@maaktweluit maaktweluit left a comment

Choose a reason for hiding this comment

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

LGTM!

One nitty comment and will approve when the threading question is solved

@@ -1163,6 +1182,30 @@ def test_already_assigned(
dispatcher_mock.send.assert_not_called()
logger_mock.error.assert_called()

def test_task_api(
self, _logger_mock, _dispatcher_mock,
Copy link
Contributor

Choose a reason for hiding this comment

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

one argument per line from the style guide

@Krigpl
Copy link
Contributor Author

Krigpl commented Aug 12, 2019

@maaktweluit I consider the threading issue resolved.

Copy link
Contributor

@maaktweluit maaktweluit left a comment

Choose a reason for hiding this comment

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

ok, thanks!

@codecov
Copy link

codecov bot commented Aug 12, 2019

Codecov Report

Merging #4561 into develop will decrease coverage by <.01%.
The diff coverage is 95.23%.

@@             Coverage Diff             @@
##           develop    #4561      +/-   ##
===========================================
- Coverage    90.33%   90.33%   -0.01%     
===========================================
  Files          225      225              
  Lines        19929    19944      +15     
===========================================
+ Hits         18003    18016      +13     
- Misses        1926     1928       +2

@mfranciszkiewicz mfranciszkiewicz merged commit ad8737e into develop Aug 12, 2019
@mfranciszkiewicz mfranciszkiewicz deleted the task_api_compute branch August 12, 2019 09:36
@mfranciszkiewicz
Copy link
Contributor

@Krigpl Please create a follow-up PR solving the sync_wait issue.

@Krigpl
Copy link
Contributor Author

Krigpl commented Aug 12, 2019

@mfranciszkiewicz Didn't you find that this runs in a separate thread anyway so it doesn't effectively matter much anyway at the moment?

@mfranciszkiewicz
Copy link
Contributor

@Krigpl ATM it runs in a separate thread through DockerTaskThread. NewTaskComputer doesn't make use of the DockerTaskThread class.

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

Successfully merging this pull request may close these issues.

4 participants