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

Created task fragments RPC #4120

Merged
merged 6 commits into from
Apr 17, 2019
Merged

Created task fragments RPC #4120

merged 6 commits into from
Apr 17, 2019

Conversation

kmazurek
Copy link
Contributor

Resolves: #4041

Adds an RPC endpoint which returns fragments for a given rendering task.
A fragment is a collection of subtasks which all refer to a common part
of the whole task (currently it is the start_task index from subtask
extra_data).

Adds an RPC endpoint which returns fragments for a given rendering task.
A fragment is a collection of subtasks which all refer to a common part
of the whole task (currently it is the start_task index from subtask
extra_data).
@kmazurek kmazurek requested review from jiivan and shadeofblue April 15, 2019 12:42
@kmazurek kmazurek self-assigned this Apr 15, 2019
@ghost ghost added the in progress label Apr 15, 2019
@kmazurek
Copy link
Contributor Author

Sample JSON response from the RPC in its current form (subtask_count is 5, only one subtask has been assigned):

{
    "1": [
        {
            "node_id": "da0853e0ec7477edbbf5fdd036093066616c5763f5aa6a670742617dc11f0e325846c53dbccba9f3aada8c6dceecf2acbbe0c44f0bfe2ec87db7cf53553a19a8",
            "node_name": "",
            "progress": 1.0,
            "results": [
                "/home/.../ComputerRes/31d06954-5c6f-11e9-8455-03db94344a05/tmp/small_shark100_10001.png",
                "/home/.../ComputerRes/31d06954-5c6f-11e9-8455-03db94344a05/tmp/small_shark100_10002.png"
            ],
            "status": "Finished",
            "stderr": "/home/.../ComputerRes/31d06954-5c6f-11e9-8455-03db94344a05/tmp/47cb889c-5c6f-11e9-8d2d-03db94344a05/stderr.log",
            "stdout": "/home/.../ComputerRes/31d06954-5c6f-11e9-8455-03db94344a05/tmp/47cb889c-5c6f-11e9-8d2d-03db94344a05/stdout.log",
            "subtask_id": "47cb889c-5c6f-11e9-8d2d-03db94344a05",
            "time_remaining": 0.0,
            "time_started": 1554996863.0686917
        }
    ],
    "2": [],
    "3": [],
    "4": [],
    "5": []
}

golem/client.py Outdated Show resolved Hide resolved
Copy link
Contributor

@shadeofblue shadeofblue left a comment

Choose a reason for hiding this comment

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

abstract-paper-fracture-detail-lasercut-paper-by-robin-hanhart

@@ -5,7 +5,7 @@
import logging
import os.path
import re
import typing
from typing import Any, Dict, Generator, Iterable, List, Optional, Tuple
Copy link
Contributor

Choose a reason for hiding this comment

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

why?... maybe it's just me but I find typing.X immediately more readable than just X ...

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 find prefixing overly verbose, especially for collection type signatures (when the prefix gets repeated multiple times). I suppose it's mostly subjective, but from typing seems to be more widely used in our codebase (113 vs 29 files).

Copy link
Contributor

Choose a reason for hiding this comment

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

I second @shadeofblue in that. IMO this is a case of namespace pollution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

surrender

golem/task/rpc.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Apr 16, 2019

Codecov Report

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

@@             Coverage Diff             @@
##           develop    #4120      +/-   ##
===========================================
+ Coverage    88.64%   88.67%   +0.03%     
===========================================
  Files          214      214              
  Lines        18682    18697      +15     
===========================================
+ Hits         16561    16580      +19     
+ Misses        2121     2117       -4

Copy link
Contributor

@jiivan jiivan left a comment

Choose a reason for hiding this comment

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

Added remote procedure looks good 👍

I don't like changing clean namespace into this polluted... thing. :/

>>> import this
The Zen of Python, by Tim Peters
[...]
Namespaces are one honking great idea -- let's do more of those!

@@ -5,7 +5,7 @@
import logging
import os.path
import re
import typing
from typing import Any, Dict, Generator, Iterable, List, Optional, Tuple
Copy link
Contributor

Choose a reason for hiding this comment

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

I second @shadeofblue in that. IMO this is a case of namespace pollution.

Copy link
Contributor

@jiivan jiivan left a comment

Choose a reason for hiding this comment

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

buakaw_throw

@kmazurek kmazurek merged commit f54145c into develop Apr 17, 2019
@ghost ghost removed the in progress label Apr 17, 2019
@kmazurek kmazurek deleted the task-fragments-rpc branch April 17, 2019 10:41
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.

Task fragments RPC
3 participants