Remove GetResources and ResourceList message

[badb]

Information about resources should be included in ComputeTaskDef. Thanks to that we can:

1. Simplify the protocol
2. Add support for sending different resources for different subtasks, which will be necessary for future use cases.

Do not merge before: golemfactory/golem-messages#308

[jiivan]

Hey, no need for all these changes. You can just replace TaskHeader() with TaskHeaderFactory(...)

[badb]

There is TaskHeaderFactory. Unfortunately most of this fields need to have specific values for the rest of the script to work properly (so they cannot be completely random).

[jiivan]

try this:

        header = TaskHeaderFactory(
              task_id=task_id,
              task_owner=task_owner,
              environment=environment,
              deadline=timeout_to_deadline(14400),
              subtask_timeout=1200,
              subtasks_count=num_subtasks,
              resource_size=params.shared_data_size + params.subtask_data_size,
              estimated_memory=0,
              max_price=MIN_PRICE,
              min_version=golem.__version__,
              timestamp=int(get_timestamp_utc()),
          )

[jiivan]

Would it be possible to avoid adding this to TaskSession instance? Like storing in Task[Manager|Keeper]? Keeping TS stateless would allow to eventually simplify establishing new Session and making a simple operation as "send msgX to nodeY" easy. #2223

[Krigpl]

This isn't needed, is it?
ctd['resources'] is supposed to contain package IDs downloadable from p2p network, not actual filepaths

[shadeofblue]

hmm... I'm unsure ... right now, ResourceList servers as a point to exchange the resources but the list is always assumed to contain just one element ... and then, when e.g. the resources are passed to the Concent, they're passed in a different way (using HTTP, not p2p) and are verified thanks to the hash contained in TTC itself ...

if now resources could comprise of multiple parts, hashes of each part would need to be specified somehow if Concent was to be able to verify each of them ...

so, the list here is not only needed, but actually, @badb the list should probably be a list of dicts (?) containing both locations (p2p IDs of those files) but also hashes of those files ...

[badb]

It's not needed yet. Will be in the next step. I can remove it and add when it's actually needed.
Right now way that resources are sent is not really much different than the previous one.
@shadeofblue Can you show me the part of the code where the resources are passed to Concent?

[shadeofblue]

hmm, I feel undecided here... on one hand, I understand this is mostly a refactor aiming to remove an unnecessary exchange of messages and in this case, I can only second @jiivan that - if possible - it shouldn't add statefulness to TaskSesssion but at the same time - I understand that it may be unfeasible at this stage to straighten-out the TaskSession in this pull request here...

On the other - I have one more uncertainty with regards to multiple-package resources and Concent - are we assuming those multi-package tasks right now just won't be supported by Concent? if so, maybe we should modify Golem not to allow the concent_enabled flag to be set on such multi-package tasks until we decide how we want them supported?

iow (in case we don't want Concent support for multi-package tasks right now):

• on Requestor's end, we'd need to respond with "CannotAssignTask" to any concent-enabled WTCTs referring to those tasks or more generally - disallow creation of concent-enabled, multi-package tasks
• on Provider's end (at this moment), ignore any TaskHeaders that are concent-enabled and refer to multi-package tasks (unsure if it's already possible to determine that such task is a multi-package task at this stage)

On the other hand, if we could probably enable the Concent to process such muti-package tasks but then as I said above - we'd need to enable the Concent to also verify those additional resources - most likely by adding package hashes somewhere in TTC or earlier... of course, we'd need to modify the Concent itself to enable such support in the service itself...

[badb]

(1) Additional statefulness of Task Session has been removed.
(2) Resource transport logic for Blender will not be changed (except of the protocol message format).
(3) In this PR resources are formatted in exactly the same way they were sent before, just in the different message.
(4) What kind of changes are exactly needed to make this PR acceptable? because I'm lost after reading this comment.

[jiivan]

lgtm

[codecov]

Codecov Report

Merging #3744 into develop will decrease coverage by 0.09%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           develop    #3744     +/-   ##
==========================================
- Coverage    87.81%   87.71%   -0.1%     
==========================================
  Files          216      216             
  Lines        19008    19009      +1     
==========================================
- Hits         16691    16673     -18     
- Misses        2317     2336     +19

[shadeofblue]

@badb I'm fine with those changes with the reservations that we discussed offline (iow, that we'll need to address those changes to TTC/TaskHeader in a future update)