Increase laziness of SerializationSettings

[madhur-tandon]

TL;DR

This PR makes passing of SerializationSettings for APIs like register_task and register_workflow lazy aka a user may not pass them. This is helpful in cases like:

1. Using a task which doesn't explicitly require a container, eg: SQLTask
2. Using a fetched task in a workflow, which is already registered and thus doesn't require the use of SerializationSettings

In cases where it is required, the error is bubbled up to translator.py which throws a ValueError at some point.

Type

• [ ] Bug Fix
• [x] Feature
• [ ] Plugin

Are all requirements met?

• [x] Code completed
• [ ] Smoke tested
• [x] Unit tests added
• [ ] Code documentation added
• [ ] Any pending items have an associated Issue

Complete description

1. The approach is to make SerializationSettings optional. But since we cannot change the API i.e. function signatures -- a dummy SerializationSettings object is created when the user doesn't pass one (i.e. it is None). This dummy object doesn't have any image associated with it. Hence, the error of not having an image is bubbled up and is handled later in translator.py

2. The following cases are tested (without passing SerializationSettings of course):

• Registering a SQL Task -- should allow
• Registering a Python based task -- should throw an error
• Registering a Workflow that only has SQL Tasks (or tasks used to fetch something etc.) -- should allow
• Registering a Workflow that uses both SQL based Tasks and Python based Tasks -- should throw an error
• Registering a Workflow only based on fetched tasks (even if they were originally Python based) -- should allow
• Registering a Workflow based on fetched tasks as well as some local Python Tasks -- should throw an error

Some other observations

1. register_launch_plan is inconsistent with register_task and register_workflow — the latter 2 have SerializationSettings in their arguments, but the former does not. In my opinion, these 3 functions should be consistent aka have similar function signatures. But this is out of scope for the given task. I have added a TODO comment.

2. Inside the function register_task, the call to _serialize_and_register can probably replaced by the following snippet:

from flytekit.tools.translator import  get_serializable_task
...
...
ident = self._resolve_identifier(ResourceType.TASK, entity.name, version, serialization_settings)
m = OrderedDict()
idl_task = get_serializable_task(m, serialization_settings, entity)
self.client.create_task(task_identifer=ident, task_spec=idl_task.spec)

Similar stuff can be done for register_workflow. But I am not sure if this is even required. But if this is indeed done, we can possibly eliminate the use of _serialize_and_register altogether. Regardless, this PR doesn't attempt to do this.

Tracking Issue

https://github.com/flyteorg/flyte/issues/2647

Follow-up issue

NA

[codecov[bot]]

Codecov Report

Merging #1085 (519b77a) into master (d560ebe) will increase coverage by 0.46%. The diff coverage is 97.43%.

@@            Coverage Diff             @@
##           master    #1085      +/-   ##
==========================================
+ Coverage   86.67%   87.14%   +0.46%     
==========================================
  Files         269      269              
  Lines       25074    25222     +148     
  Branches     2826     2837      +11     
==========================================
+ Hits        21734    21979     +245     
+ Misses       2871     2769     -102     
- Partials      469      474       +5     

Impacted Files	Coverage Δ
flytekit/core/python_function_task.py	87.77% <ø> (ø)
tests/flytekit/unit/remote/test_wrapper_classes.py	82.22% <80.00%> (ø)
flytekit/tools/translator.py	85.96% <93.33%> (+1.75%)	:arrow_up:
tests/flytekit/unit/remote/test_remote.py	98.66% <95.91%> (-1.34%)	:arrow_down:
tests/flytekit/unit/remote/test_with_responses.py	98.73% <96.87%> (-1.27%)	:arrow_down:
flytekit/remote/remote.py	51.34% <100.00%> (+10.23%)	:arrow_up:
flytekit/tools/serialize_helpers.py	92.75% <100.00%> (+23.18%)	:arrow_up:
tests/flytekit/unit/core/test_conditions.py	98.77% <100.00%> (ø)
tests/flytekit/unit/core/test_flyte_pickle.py	91.11% <100.00%> (ø)
tests/flytekit/unit/core/test_imperative.py	99.15% <100.00%> (ø)
... and 26 more

---

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 d560ebe...519b77a. Read the comment docs.