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 pipeline component examples catalogs for KFP and AA and MLX connector #79

Merged
merged 18 commits into from
Nov 12, 2021

Conversation

ptitzler
Copy link
Member

@ptitzler ptitzler commented Nov 5, 2021

This PR

Add examples catalog user experience (Airflow shown):

  • Open "Manage Components"

  • Add catalog (+)

  • New Apache Airflow example components catalog

  • Enter a catalog name (any string) - all other inputs are pre-populated.
    image

    Note that the Learn more... link opens the connector's GitHub repository README, which contains information about the connector. (For built-in catalog types the link opens the pipeline components topic in the user guide, which introduces catalogs.)

  • Expand palette in VPE
    image

Note that the following information is persisted for each node in the .pipeline file (irrelevant information was removed from the KFP snippet below):

"type": "execution_node",
"op": "elyra-kfp-examples-catalog:737915b826e9",
"component_description": "Filter input text according to the given regex pattern using shell and grep.",
"component_source": "{'catalog_type': 'elyra-kfp-examples-catalog', 'component_ref': {'component-id': 'filter_text_using_shell_and_grep.yaml'}}",

Airflow:

"type": "execution_node",
"op": "elyra-airflow-examples-catalog:3a55d015ea96",
"component_source": "{'catalog_type': 'elyra-airflow-examples-catalog', 'component_ref': {'component-id': 'bash_operator.py'}}",

Developer's Certificate of Origin 1.1

   By making a contribution to this project, I certify that:

   (a) The contribution was created in whole or in part by me and I
       have the right to submit it under the Apache License 2.0; or

   (b) The contribution is based upon previous work that, to the best
       of my knowledge, is covered under an appropriate open source
       license and I have the right under that license to submit that
       work with modifications, whether created in whole or in part
       by me, under the same open source license (unless I am
       permitted to submit under a different license), as indicated
       in the file; or

   (c) The contribution was provided directly to me by some other
       person who certified (a), (b) or (c) and I have not modified
       it.

   (d) I understand and agree that this project and the contribution
       are public and that a record of the contribution (including all
       personal information I submit with it, including my sign-off) is
       maintained indefinitely and may be redistributed consistent with
       this project or the open source license(s) involved.

@lresende
Copy link
Member

lresende commented Nov 7, 2021

Please make sure to update LICENSE files accordingly to mention new files as in PR 2272

Copy link
Member

@kiersten-stokes kiersten-stokes left a comment

Choose a reason for hiding this comment

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

This is working great! I installed from source for my tests

@ptitzler
Copy link
Member Author

ptitzler commented Nov 8, 2021

A couple pending changes:

  • split connector into two connectors (one for kfp, one for airflow):
    • avoids issues like the ones we are facing with other BYOs (stuff that doesn't belong together is stored together) -> this will make it easier for somebody to create an "examples" connector for another runtime type
    • customized schema, which pre-populates as much as possible, e.g. for KFP
    • add connector
    • image
    • followed by "enable connector"
    • image

Note:

  • the elyra setup.py will list both connectors as extras
  • the Dockerfile templates we include in the elyra repo will install [the relevant] connector package(s) AND invoke elyra-metadata install .... This way the example components are available in the VPE palette by default. Since these are just templates, customers can choose to remove the dependencies/setup steps in their customized version if they want to exclude the example components (or use their own "forked" examples instead.

@kevin-bates
Copy link
Member

The updates sound great. I still think it would be good to expose a filter/regex schema property that can be applied so users can build connector instances that are subsets of the complete set.

@ptitzler
Copy link
Member Author

ptitzler commented Nov 8, 2021

Please make sure to update LICENSE files accordingly to mention new files as in PR 2272

Thank you, I have totally missed that. I added a license file to the Kubeflow Pipelines connector's resources directory. The Airflow components already have their own license embedded, so we should be good there!

@ptitzler ptitzler changed the title Add pipeline component examples catalog connector Add pipeline component examples catalog connectors for KFP and AA Nov 8, 2021
@kevin-bates
Copy link
Member

Sorry - I just noticed my comments were applied to a specific commit, which requires you to have to open the commit link to get their context. What a hassle - I apologize.

Copy link
Member

@kevin-bates kevin-bates left a comment

Choose a reason for hiding this comment

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

Aside from the circular ref comment below, the connector schemas should include the following updates (can't apply in-line commenting so adding here):

  • The $id values still reflect the combined form of the repo
  • The display_name property should be removed as it's redundant with title. (This comment is NOT apply to the display_name property definition down in the properties stanza - which applies to instances.) All schemas are being updated in this manner in Remove references to schema display_name property elyra#2267
  • The schemaspace and schemaspace_id values should be updated to component-catalogs and 8dc89ca3-4b90-41fd-adb9-9510ad346620, respectively once Component catalogs schemaspace elyra#2282 is merged.

@ptitzler
Copy link
Member Author

ptitzler commented Nov 9, 2021

[...] the connector schemas should include the following updates (can't apply in-line commenting so adding here):

  • The $id values still reflect the combined form of the repo

Fixed in both schemas!

  • The display_name property should be removed as it's redundant with title. (This comment is NOT apply to the display_name property definition down in the properties stanza - which applies to instances.) All schemas are being updated in this manner in elyra-ai/elyra#2267

Removed the top level display_name property from both schemas!

  • The schemaspace and schemaspace_id values should be updated to component-catalogs and 8dc89ca3-4b90-41fd-adb9-9510ad346620, respectively once elyra-ai/elyra#2282 is merged.

Adding this to the TODO list

@ptitzler ptitzler changed the title Add pipeline component examples catalog connectors for KFP and AA Add pipeline component examples catalogs for KFP and AA and MLX connector Nov 9, 2021
Copy link
Member

@kiersten-stokes kiersten-stokes left a comment

Choose a reason for hiding this comment

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

LGTM! I incorporated the changes that this PR is dependent on and tested all three catalog types -- all are working great

@ptitzler
Copy link
Member Author

  • chemaspace and schemaspace_id values should be updated to component-catalogs and 8dc89ca3-4b90-41fd-adb9-9510ad346620, respectively once elyra-ai/elyra#2282 is merged.

Done!

@ptitzler
Copy link
Member Author

@akchinSTC @lresende @kevin-bates @kiersten-stokes you should have received pending collaborator invitations for three packages from PyPI

@ptitzler
Copy link
Member Author

For test purposes version 0.0.1 of each package was published to unblock elyra-ai/elyra#2286. Once development has completed and no issues were found version 0.0.1 will be replaced by 0.1.0.

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.

5 participants