docs(KEP): add proposal for consolidating the python packages#12548
docs(KEP): add proposal for consolidating the python packages#12548zazulam wants to merge 1 commit intokubeflow:masterfrom
Conversation
|
Regarding the tfx question that was brought up in the last community call:
From searching against the tfx repo, it looks like |
droctothorpe
left a comment
There was a problem hiding this comment.
/lgtm
Really excited for this improvement. Kudos, @zazulam!
|
@chensun could you please review this? |
| This will install the consolidated package. The old packages (`kfp-pipeline-spec`, etc.) will no longer be needed as dependencies, though they may remain on PyPI for older versions. | ||
|
|
||
| **Breaking Changes**: | ||
| * Users directly importing `kfp_pipeline_spec`, `kfp_server_api`, or `kfp_kubernetes` will need to update their imports to `kfp.pipeline_spec`, `kfp.server_api`, and `kfp.kubernetes` respectively. |
There was a problem hiding this comment.
kfp_pipeline_spec and kfp_kubernetes are kfp-namespaced package already. The imports are already kfp.pipeline_spec and kfp.kubernetes there's no change how they are imported. Only change is how they're installed.
|
|
||
| **Question**: Should `kfp.kubernetes` be an optional dependency? | ||
|
|
||
| **Recommendation**: **Yes**. |
There was a problem hiding this comment.
+1, another factor is that Vertex customers don't use kfp.kubernetes.
| 1. **Unit Tests**: Migrate existing unit tests from the separate repositories/directories into the `kfp` test suite. | ||
| * Specifically, `kubernetes_platform` tests will be moved to `sdk/python/kfp/kubernetes/test/`. | ||
| 2. **Integration Tests**: Verify that the consolidated package works with existing integration tests. | ||
| 3. **Installation Tests**: Verify that `pip install .` and `pip install kfp` work correctly in a fresh environment. |
There was a problem hiding this comment.
I don't know if this may be handled magically by pip. We should test the behavior of upgrading kfp from an older version with the extension packages installed to a newer version where extensions are included in the main kfp package, and see if the the old extension packages make cause any conflicts. I recall running into errors caused by incompatibility issues between legacy and updated modules, and when that happens the error message are sometimes confusing and not easy to debug.
2a8cc21 to
f6f8c3d
Compare
f6f8c3d to
1ced926
Compare
Signed-off-by: zazulam <m.zazula@gmail.com>
1ced926 to
30d9150
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: droctothorpe The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description of your changes:
KEP for merging the packages into one.
Related: #12644
Checklist: