Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add new control plane classes #425

Merged
merged 8 commits into from
Apr 28, 2021
Merged

add new control plane classes #425

merged 8 commits into from
Apr 28, 2021

Conversation

cosmicBboy
Copy link
Contributor

@cosmicBboy cosmicBboy commented Mar 22, 2021

Signed-off-by: cosmicBboy [email protected]

TL;DR

Add Flyte* control plane objects to give programmatic access to flyte backend data structures.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

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

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

flyteorg/flyte#823

Follow-up issue

NA
OR
https://github.com/lyft/flyte/issues/

@codecov
Copy link

codecov bot commented Mar 22, 2021

Codecov Report

Merging #425 (ef22e9e) into master (d77a773) will decrease coverage by 0.04%.
The diff coverage is 71.70%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
- Coverage   84.39%   84.35%   -0.05%     
==========================================
  Files         344      357      +13     
  Lines       25835    26608     +773     
  Branches     2114     2205      +91     
==========================================
+ Hits        21804    22445     +641     
- Misses       3458     3564     +106     
- Partials      573      599      +26     
Impacted Files Coverage Δ
flytekit/control_plane/tasks/executions.py 34.28% <34.28%> (ø)
flytekit/control_plane/component_nodes.py 56.45% <56.45%> (ø)
flytekit/control_plane/nodes.py 56.84% <56.84%> (ø)
flytekit/control_plane/workflow.py 66.66% <66.66%> (ø)
flytekit/control_plane/workflow_execution.py 70.58% <70.58%> (ø)
flytekit/control_plane/launch_plan.py 76.08% <76.08%> (ø)
flytekit/control_plane/interface.py 90.00% <90.00%> (ø)
flytekit/control_plane/tasks/task.py 93.61% <93.61%> (ø)
...lytekit/integration/control_plane/test_workflow.py 97.87% <97.87%> (ø)
flytekit/control_plane/identifier.py 100.00% <100.00%> (ø)
... and 28 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 d77a773...ef22e9e. Read the comment docs.

flytekit/control_plane/workflow.py Outdated Show resolved Hide resolved
flytekit/control_plane/workflow.py Outdated Show resolved Hide resolved
flytekit/control_plane/workflow.py Show resolved Hide resolved
flytekit/control_plane/launch_plan.py Show resolved Hide resolved
flytekit/control_plane/component_nodes.py Outdated Show resolved Hide resolved
flytekit/control_plane/identifier.py Outdated Show resolved Hide resolved
flytekit/control_plane/launch_plan.py Outdated Show resolved Hide resolved
flytekit/control_plane/launch_plan.py Outdated Show resolved Hide resolved
flytekit/control_plane/nodes.py Outdated Show resolved Hide resolved
flytekit/control_plane/nodes.py Outdated Show resolved Hide resolved
flytekit/control_plane/nodes.py Show resolved Hide resolved
flytekit/control_plane/nodes.py Show resolved Hide resolved
flytekit/control_plane/notifications.py Outdated Show resolved Hide resolved
@cosmicBboy cosmicBboy force-pushed the control-plane branch 2 times, most recently from f632525 to d5dd70d Compare April 22, 2021 20:22
@cosmicBboy cosmicBboy changed the title [WIP] add FlyteTask control_plane class add new control plane classes Apr 22, 2021
Signed-off-by: cosmicBboy <[email protected]>
Signed-off-by: cosmicBboy <[email protected]>
flytekit/control_plane/workflow.py Outdated Show resolved Hide resolved
self._executable_flyte_object = flyte_task or flyte_workflow or flyte_launch_plan
if parameter_mapping:
if not flyte_branch:
self._outputs = OutputParameterMapper(self._executable_flyte_object.interface.outputs, self)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we remove this for now? this is old stuff.

flytekit/control_plane/launch_plan.py Outdated Show resolved Hide resolved
flytekit/control_plane/launch_plan.py Outdated Show resolved Hide resolved
return False

@property
def auth_role(self) -> _common_models.AuthRole:
Copy link
Contributor

Choose a reason for hiding this comment

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

is this function necessary if we're just reading from the template?

flytekit/control_plane/interface.py Outdated Show resolved Hide resolved
@wild-endeavor
Copy link
Contributor

If you just do

---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-3-411e700e2550> in <module>
----> 1 FlyteWorkflowExecution.fetch('flytesnacks', 'development', 'upu1b8xod3')

~/go/src/github.com/flyteorg/flytekit/flytekit/control_plane/workflow_execution.py in fetch(cls, project, domain, name)
    122     def fetch(cls, project: str, domain: str, name: str) -> "FlyteWorkflowExecution":
    123         return cls.promote_from_model(
--> 124             _flyte_engine.get_client().get_execution(
    125                 _core_identifier.WorkflowExecutionIdentifier(project=project, domain=domain, name=name)
    126             )

~/go/src/github.com/flyteorg/flytekit/flytekit/engines/flyte/engine.py in get_client()
     54 # This will be refactored away when we move to a heavier context object.
     55 def get_client() -> _SynchronousFlyteClient:
---> 56     return _FlyteClientManager(_platform_config.URL.get(), insecure=_platform_config.INSECURE.get()).client
     57
     58

~/go/src/github.com/flyteorg/flytekit/flytekit/engines/flyte/engine.py in __init__(self, *args, **kwargs)
     40         # TODO: use cases.
     41         if type(self)._CLIENT is None:
---> 42             c = _SynchronousFlyteClient(*args, **kwargs)
     43             type(self)._CLIENT = c
     44

~/go/src/github.com/flyteorg/flytekit/flytekit/clients/raw.py in __init__(self, url, insecure, credentials, options)
    151             self._channel = _insecure_channel(url, options=list((options or {}).items()))
    152         else:
--> 153             self._channel = _secure_channel(
    154                 url,
    155                 credentials or _ssl_channel_credentials(),

~/envs/flytekit/lib/python3.8/site-packages/grpc/__init__.py in secure_channel(target, credentials, options, compression)
   1968             "secure_channel cannot be called with insecure credentials." +
   1969             " Call insecure_channel instead.")
-> 1970     return _channel.Channel(target, () if options is None else options,
   1971                             credentials._credentials, compression)
   1972

~/envs/flytekit/lib/python3.8/site-packages/grpc/_channel.py in __init__(self, target, options, credentials, compression)
   1477         self._process_python_options(python_options)
   1478         self._channel = cygrpc.Channel(
-> 1479             _common.encode(target), _augment_options(core_options, compression),
   1480             credentials)
   1481         self._call_state = _ChannelCallState(self._channel)

~/envs/flytekit/lib/python3.8/site-packages/grpc/_common.py in encode(s)
     70         return s
     71     else:
---> 72         return s.encode('utf8')
     73
     74

AttributeError: 'NoneType' object has no attribute 'encode'

you just get a funny error. I ended up having to call:

FLYTE_PLATFORM_URL=localhost:30081 FLYTE_PLATFORM_INSECURE=True ipython

I think we need a better way to specify settings. We should probably start by respecting ~/.flyte/config if there's a file there.

@wild-endeavor
Copy link
Contributor

Also, just the fact that I can get network setting related errors when calling fetch is making me lean towards the centralized idea we talked about.

@cosmicBboy
Copy link
Contributor Author

you just get a funny error. I ended up having to call:

FLYTE_PLATFORM_URL=localhost:30081 FLYTE_PLATFORM_INSECURE=True ipython

I think we need a better way to specify settings. We should probably start by respecting ~/.flyte/config if there's a file there.

agreed, shall I make an issue for that?

also, let me know if this is good to merge

@cosmicBboy cosmicBboy merged commit 31538ba into master Apr 28, 2021
@cosmicBboy cosmicBboy deleted the control-plane branch April 28, 2021 20:47
max-hoffman pushed a commit to dolthub/flytekit that referenced this pull request Apr 29, 2021
* implement new control plane classes

Signed-off-by: cosmicBboy <[email protected]>

* revert dep changes

Signed-off-by: cosmicBboy <[email protected]>

* remove unneeded mock integration test files

Signed-off-by: cosmicBboy <[email protected]>

* remove pytest.ini

Signed-off-by: cosmicBboy <[email protected]>

* add integration tests to ci, update reqs

Signed-off-by: cosmicBboy <[email protected]>

* add unit tests

Signed-off-by: cosmicBboy <[email protected]>

* lint

Signed-off-by: cosmicBboy <[email protected]>

* address comments @wild-endeavor

Signed-off-by: cosmicBboy <[email protected]>
Signed-off-by: Max Hoffman <[email protected]>
max-hoffman pushed a commit to dolthub/flytekit that referenced this pull request Apr 29, 2021
* implement new control plane classes

Signed-off-by: cosmicBboy <[email protected]>

* revert dep changes

Signed-off-by: cosmicBboy <[email protected]>

* remove unneeded mock integration test files

Signed-off-by: cosmicBboy <[email protected]>

* remove pytest.ini

Signed-off-by: cosmicBboy <[email protected]>

* add integration tests to ci, update reqs

Signed-off-by: cosmicBboy <[email protected]>

* add unit tests

Signed-off-by: cosmicBboy <[email protected]>

* lint

Signed-off-by: cosmicBboy <[email protected]>

* address comments @wild-endeavor

Signed-off-by: cosmicBboy <[email protected]>
Signed-off-by: Max Hoffman <[email protected]>
max-hoffman pushed a commit to dolthub/flytekit that referenced this pull request Apr 29, 2021
* implement new control plane classes

Signed-off-by: cosmicBboy <[email protected]>

* revert dep changes

Signed-off-by: cosmicBboy <[email protected]>

* remove unneeded mock integration test files

Signed-off-by: cosmicBboy <[email protected]>

* remove pytest.ini

Signed-off-by: cosmicBboy <[email protected]>

* add integration tests to ci, update reqs

Signed-off-by: cosmicBboy <[email protected]>

* add unit tests

Signed-off-by: cosmicBboy <[email protected]>

* lint

Signed-off-by: cosmicBboy <[email protected]>

* address comments @wild-endeavor

Signed-off-by: cosmicBboy <[email protected]>
Signed-off-by: Max Hoffman <[email protected]>
max-hoffman pushed a commit to dolthub/flytekit that referenced this pull request Apr 29, 2021
* implement new control plane classes

Signed-off-by: cosmicBboy <[email protected]>

* revert dep changes

Signed-off-by: cosmicBboy <[email protected]>

* remove unneeded mock integration test files

Signed-off-by: cosmicBboy <[email protected]>

* remove pytest.ini

Signed-off-by: cosmicBboy <[email protected]>

* add integration tests to ci, update reqs

Signed-off-by: cosmicBboy <[email protected]>

* add unit tests

Signed-off-by: cosmicBboy <[email protected]>

* lint

Signed-off-by: cosmicBboy <[email protected]>

* address comments @wild-endeavor

Signed-off-by: cosmicBboy <[email protected]>
Signed-off-by: Max Hoffman <[email protected]>
max-hoffman pushed a commit to dolthub/flytekit that referenced this pull request May 11, 2021
* implement new control plane classes

Signed-off-by: cosmicBboy <[email protected]>

* revert dep changes

Signed-off-by: cosmicBboy <[email protected]>

* remove unneeded mock integration test files

Signed-off-by: cosmicBboy <[email protected]>

* remove pytest.ini

Signed-off-by: cosmicBboy <[email protected]>

* add integration tests to ci, update reqs

Signed-off-by: cosmicBboy <[email protected]>

* add unit tests

Signed-off-by: cosmicBboy <[email protected]>

* lint

Signed-off-by: cosmicBboy <[email protected]>

* address comments @wild-endeavor

Signed-off-by: cosmicBboy <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants