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.

Sign up for GitHub

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

RFC: Community plugins #5610

Merged
merged 9 commits into from
Oct 18, 2024
Merged

RFC: Community plugins #5610

Changes from 3 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3fad42c
Create RFC for community plugins
davidmirror-ops Jul 31, 2024
87a55fa
Apply part of review
davidmirror-ops Aug 1, 2024
17ed543
Update promotion process
davidmirror-ops Aug 28, 2024
e7efe71
Update rfc/system/0008-community-plugins.md
fg91 Aug 29, 2024
ee2d397
Remove codeowners step
davidmirror-ops Sep 25, 2024
a77d103
Update rfc/system/0008-community-plugins.md
fg91 Sep 26, 2024
4a4cb82
Update rfc/system/0008-community-plugins.md
fg91 Sep 26, 2024
d7de8de
Update rfc/system/0008-community-plugins.md
fg91 Sep 26, 2024
a72533b
Apply suggestions from code review
davidmirror-ops Sep 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions rfc/system/0008-community-plugins.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# Management of community-contributed plugins

**Authors:**

- @davidmirror-ops


## 1 Executive Summary

The Flyte community as a self-governed and productive collective of individuals, values contributions. This proposal aims to discuss the process community contributors should follow to submit a new `flytekit` plugin, with special attention to mechanisms that ensure stability and maintainability of core flyte code.

## 2 Motivation

- With the current "in-tree" approach, plugins developed by the community land on the `flytekit` repo ([example](https://github.com/flyteorg/flytekit/pull/2537)). It results in Flyte maintainers having to take care of CI test failures due to plugin code or flytekit updates incompatible with plugin code, etc.
davidmirror-ops marked this conversation as resolved.
Show resolved Hide resolved

- The goal is to agree on a process for contributors to follow when submitting new integrations in a "out-of-tree" way that clearly communicates that it is a community-contributed -and then- community-supported integration.

## 3 Proposed Implementation

- Create a `community` folder under `flytekit/plugins` and keep releasing the plugins in that folder as separate `pypi` packages.
- Configure CI to only run tests on `plugins/community` when there are changes.
fg91 marked this conversation as resolved.
Show resolved Hide resolved
- Keep releasing plugins alongside flytekit.
- Update [CODEOWNERS](https://github.com/flyteorg/flytekit/blob/master/CODEOWNERS) for the `plugins/community` folder, enabling contributors to merge community plugins (it requires granting Write permissions to contributors on `flytekit`).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Update [CODEOWNERS](https://github.com/flyteorg/flytekit/blob/master/CODEOWNERS) for the `plugins/community` folder, enabling contributors to merge community plugins (it requires granting Write permissions to contributors on `flytekit`).
- Update [CODEOWNERS](https://github.com/flyteorg/flytekit/blob/master/CODEOWNERS) for the `plugins/community` folder so that plugin authors are tagged for PR reviews.
- Enable contributors to merge community plugins (it requires granting Write permissions to contributors on `flytekit`).

Copy link
Member

Choose a reason for hiding this comment

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

Let's talk about this point today. Will every plugin author automatically get the permission to approve and merge PRs in the entire repo? Github's permissions model is a bit limiting for us here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Revisiting this: how the contribution process would look like? Shouldn't be the existing CODEOWNERS (a.k.a. flytekit maintainers) who review and merge a new community plugin? In such case, we wouldn't need to update CODEOWNERS for every new plugin contribution.
Otherwise with the Github permission model, any new plugin contributor would have to be granted write permissions to the entire flytekit repo.
cc @eapolinario

Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't be the existing CODEOWNERS (a.k.a. flytekit maintainers) who review and merge a new community plugin?

Yes, a new plugin should be merged by existing maintainers. The question for me is rather what happens after a plugin has been merged and needs to be maintained by the authors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What we talked with Eduardo today is that, considering the implication of CODEOWNERS, we would go the regular contribution path: plugin authors would have non-binding approval rights on changes to their plugins, and flytekit maintainers issue the final approvals and merge. We'd also keep releasing community plugins with flytekit, even if there are no changes. Does that make sense?

- Explicitly mark them as community maintained in the import via `import flytekitplugins.contrib.x`

fg91 marked this conversation as resolved.
Show resolved Hide resolved
### Promotion process to official plugin

An official plugin is one that is maintained by the core Flyte team and is made part of the official `flytekit` documentation.

- Plugins maintainers or community members can propose the promotion of a plugin to official by creating an Issue on the `flytekit` repo.
fg91 marked this conversation as resolved.
Show resolved Hide resolved
- The supermajority of the TSC must approve publicly before promoting a plugin.

To consider it for promotion, a plugin must meet the following criteria:

- Production readiness testing performed by the core Flyte team or documented by plugin users or maintainers
- Evidence of ongoing usage through Github issues or Slack threads

davidmirror-ops marked this conversation as resolved.
Show resolved Hide resolved
fg91 marked this conversation as resolved.
Show resolved Hide resolved


davidmirror-ops marked this conversation as resolved.
Show resolved Hide resolved
## 4 Drawbacks

- There's a management overhead associated with this initiative:
- For every new plugin contributor, we may need to update `CODEOWNERS`
- CI configuration changes in flytekit (probably a one-time change)
Copy link
Contributor

Choose a reason for hiding this comment

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

we'll need to rearrange a few things, but yeah, definitely a one-time small change.


## 5 Alternatives

- Maintain community plugins on a separate repo
- Against the monorepo initiative
- Have community packages be it's own org
- Significantly higher management overhead
- `flytekit` plugins built into their own package
- Potentially heavier development process

## 6 Open questions

- Does this apply to Agents?
Copy link
Member

Choose a reason for hiding this comment

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

it should. many people contribute agents as well

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Does this apply to Agents?
-



Loading