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 entrypoint to setup.py in flytekit plugins #1120

Merged
merged 1 commit into from
Aug 4, 2022
Merged

Conversation

pingsutw
Copy link
Member

@pingsutw pingsutw commented Aug 3, 2022

Signed-off-by: Kevin Su [email protected]

TL;DR

Polars and WhyLogs plugins add Type plugins but these are not automatically loaded as entrypoint is not specified.

This document provides how to add autoloading plugins
https://github.com/flyteorg/flytekit/tree/master/plugins

Add entrypoint to setup.py so we can implicitly load the plugin.

def load_implicit_plugins():
"""
This method allows loading all plugins that have the entrypoint specification. This uses the plugin loading
behavior as explained `here <>`_.
This is an opt in system and plugins that have an implicit loading requirement should add the implicit loading
entrypoint specification to their setup.py. The following example shows how we can autoload a module called fsspec
(whose init files contains the necessary plugin registration step)
.. code-block::
# note the group is always ``flytekit.plugins``
setup(
...
entry_points={'flytekit.plugins’: 'fsspec=flytekitplugins.fsspec'},
...
)
This works as long as the fsspec module has
.. code-block::
# For data persistence plugins
DataPersistencePlugins.register_plugin(f"{k}://", FSSpecPersistence, force=True)
# OR for type plugins
TypeEngine.register(PanderaTransformer())
# etc

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

Related to https://unionai.slack.com/archives/C038QSMN3K7/p1659539123687589

@codecov
Copy link

codecov bot commented Aug 3, 2022

Codecov Report

Merging #1120 (bd938ea) into master (e407a9d) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1120   +/-   ##
=======================================
  Coverage   68.23%   68.23%           
=======================================
  Files         287      287           
  Lines       25786    25786           
  Branches     2882     2882           
=======================================
  Hits        17595    17595           
  Misses       7713     7713           
  Partials      478      478           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@pingsutw pingsutw merged commit 9b3aa0e into master Aug 4, 2022
eapolinario pushed a commit that referenced this pull request Sep 19, 2022
eapolinario added a commit that referenced this pull request Sep 19, 2022
Signed-off-by: Kevin Su <[email protected]>

Signed-off-by: Kevin Su <[email protected]>
Co-authored-by: Kevin Su <[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