Added configArtifactRepository method into `sdk/python/kfp/compiler/compiler.py` #765

DmitryBe · 2019-02-01T05:18:20Z

Added configArtifactRepository method into sdk/python/kfp/compiler/compiler.py

Example:

# config compiler (use s3)
cpl = compiler.Compiler() \
    .configArtifactRepository(
        endpoint='s3.amazonaws.com',
        bucket='<bucket>', 
        keyPrefix='pipelines/dev',
        accessKey='', secretKey=''
    )

pipeline_filename = pipeline_func.__name__ + '.pipeline.tar.gz'
cpl.compile(pipeline_func, pipeline_filename)

This change is

k8s-ci-robot · 2019-02-01T05:18:23Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: gaoning777

If they are not already assigned, you can assign the PR to them by writing /assign @gaoning777 in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

sdk/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-ci-robot · 2019-02-01T05:18:33Z

Hi @DmitryBe. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2019-02-01T05:18:33Z

Hi @DmitryBe. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

DmitryBe · 2019-02-01T05:32:13Z

/assign @gaoning777

sdk/python/kfp/compiler/compiler.py

sdk/python/Makefile

sdk/python/kfp/_client.py

DmitryBe · 2019-02-02T08:14:51Z

Hi @Ark-kun,

Thank you for your review. To make it clear, in order to add AWS S3 support for storing artifacts, I have made 3 PR in total:

backend extend (Added AWS S3 support for storing artefacts. #766)
frontend extend (Frontend aws s3 #767)
the current one, python DSL extend.

To extend python DSL, i have added configArtifactRepository method, to parametrize method _build_conventional_artifact (default minio values where hardcoded previously).

…ompiler.py`

DmitryBe · 2019-02-10T14:37:59Z

Hi @Ark-kun,
I cleaned PR based on your suggestions, left only compiler related changes (method configArtifactRepository to parametrize _build_conventional_artifact)

Ark-kun · 2019-03-07T21:40:26Z

sdk/python/kfp/compiler/compiler.py

+  def configArtifactRepository(self, endpoint='minio-service.kubeflow:9000', bucket='mlpipeline', keyPrefix='runs', accessKey='accesskey', secretKey='secretkey', insecure=True):
+    """Config artifact repository
+    """
+    self.artifactRepositoryConfig = Compiler.ArtifactRepositoryConfig(


Can you make this field private (leading underscore)?

Ark-kun · 2019-03-07T21:41:05Z

sdk/python/kfp/compiler/compiler.py

@@ -557,6 +584,19 @@ def _compile(self, pipeline_func):
    workflow = self._create_pipeline_workflow(args_list_with_defaults, p)
    return workflow

+  def configArtifactRepository(self, endpoint='minio-service.kubeflow:9000', bucket='mlpipeline', keyPrefix='runs', accessKey='accesskey', secretKey='secretkey', insecure=True):


Python uses snake_case for function names. def set_artifact_repository(...)

Ark-kun · 2019-03-07T21:44:47Z

@IronPan @DmitryBe Do you think that it would be better to configure this at the cluster level and not bake this information into the pipeline file?

I think a better solution would be for the backend to read the default artifact repository information from cluster's ConfigMap and add it to the workflow at submission time. WDYT?

Ark-kun · 2019-04-04T22:23:43Z

I've also noticed that this configuration is S3-only while Argo supports more artifact storage solutions. We can either indicate that the config is S3-only in the name or make a more generic solution.

/ok-to-test

vicaire · 2019-04-24T23:57:01Z

@DmitryBe, @Ark-kun, what is the status on this PR? Is it ready for another round of review? Thanks.

Ark-kun · 2019-04-30T00:35:10Z

BTW, I had a PR where I've made the artifact storage configurable on the cluster level and made the compiled pipelines portable: #513

k8s-ci-robot · 2019-06-18T13:34:05Z

@DmitryBe: The following tests failed, say /retest to rerun them all:

Test name	Commit	Details	Rerun command
kubeflow-pipeline-e2e-test	`279c016`	link	`/test kubeflow-pipeline-e2e-test`
kubeflow-pipeline-sample-test	`279c016`	link	`/test kubeflow-pipeline-sample-test`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

Ark-kun · 2019-07-25T23:52:14Z

The support for setting the artifact storage options was merged in different PRs.

Ark-kun · 2019-07-26T00:56:55Z

This functionality has already been merged.

k8s-ci-robot requested review from Ark-kun and hongye-sun February 1, 2019 05:18

k8s-ci-robot requested a review from gaoning777 February 1, 2019 05:18

k8s-ci-robot added the size/M label Feb 1, 2019

k8s-ci-robot added needs-ok-to-test labels Feb 1, 2019

k8s-ci-robot assigned gaoning777 Feb 1, 2019

Ark-kun reviewed Feb 2, 2019

View reviewed changes

sdk/python/kfp/compiler/compiler.py Show resolved Hide resolved

Ark-kun reviewed Feb 2, 2019

View reviewed changes

sdk/python/Makefile Outdated Show resolved Hide resolved

Ark-kun reviewed Feb 2, 2019

View reviewed changes

sdk/python/kfp/_client.py Outdated Show resolved Hide resolved

k8s-ci-robot added size/L size/M and removed size/M size/L labels Feb 10, 2019

Added configArtifactRepository method into `sdk/python/kfp/compiler/c…

279c016

…ompiler.py`

DmitryBe changed the title ~~Updated dsl python Compiler. Added ArtifactRepositoryConfig,~~ Added configArtifactRepository method into sdk/python/kfp/compiler/compiler.py Feb 10, 2019

Ark-kun reviewed Mar 7, 2019

View reviewed changes

k8s-ci-robot added ok-to-test and removed needs-ok-to-test labels Apr 4, 2019

gaoning777 assigned Ark-kun Apr 24, 2019

Ark-kun closed this Jul 26, 2019

Linchin pushed a commit to Linchin/pipelines that referenced this pull request Apr 11, 2023

Add support to check if ingress is ready (kubeflow#765)

7d66c21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added configArtifactRepository method into `sdk/python/kfp/compiler/compiler.py` #765

Added configArtifactRepository method into `sdk/python/kfp/compiler/compiler.py` #765

DmitryBe commented Feb 1, 2019 •

edited

Loading

k8s-ci-robot commented Feb 1, 2019

k8s-ci-robot commented Feb 1, 2019

k8s-ci-robot commented Feb 1, 2019

DmitryBe commented Feb 1, 2019

DmitryBe commented Feb 2, 2019 •

edited

Loading

DmitryBe commented Feb 10, 2019

Ark-kun Mar 7, 2019

Ark-kun Mar 7, 2019

Ark-kun commented Mar 7, 2019 •

edited

Loading

Ark-kun commented Apr 4, 2019 •

edited

Loading

vicaire commented Apr 24, 2019

Ark-kun commented Apr 30, 2019

k8s-ci-robot commented Jun 18, 2019

Ark-kun commented Jul 25, 2019

Ark-kun commented Jul 26, 2019

Added configArtifactRepository method into sdk/python/kfp/compiler/compiler.py #765

Added configArtifactRepository method into sdk/python/kfp/compiler/compiler.py #765

Conversation

DmitryBe commented Feb 1, 2019 • edited Loading

k8s-ci-robot commented Feb 1, 2019

k8s-ci-robot commented Feb 1, 2019

k8s-ci-robot commented Feb 1, 2019

DmitryBe commented Feb 1, 2019

DmitryBe commented Feb 2, 2019 • edited Loading

DmitryBe commented Feb 10, 2019

Ark-kun Mar 7, 2019

Choose a reason for hiding this comment

Ark-kun Mar 7, 2019

Choose a reason for hiding this comment

Ark-kun commented Mar 7, 2019 • edited Loading

Ark-kun commented Apr 4, 2019 • edited Loading

vicaire commented Apr 24, 2019

Ark-kun commented Apr 30, 2019

k8s-ci-robot commented Jun 18, 2019

Ark-kun commented Jul 25, 2019

Ark-kun commented Jul 26, 2019

Added configArtifactRepository method into `sdk/python/kfp/compiler/compiler.py` #765

Added configArtifactRepository method into `sdk/python/kfp/compiler/compiler.py` #765

DmitryBe commented Feb 1, 2019 •

edited

Loading

DmitryBe commented Feb 2, 2019 •

edited

Loading

Ark-kun commented Mar 7, 2019 •

edited

Loading

Ark-kun commented Apr 4, 2019 •

edited

Loading