-
Notifications
You must be signed in to change notification settings - Fork 301
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
[FlyteClient][FlyteDeck] Get Downloaded Artifact Signed URL via Data Proxy #2777
[FlyteClient][FlyteDeck] Get Downloaded Artifact Signed URL via Data Proxy #2777
Conversation
Signed-off-by: Future-Outlier <eric901201@gmail.com>
Signed-off-by: Future-Outlier <eric901201@gmail.com>
Signed-off-by: Future-Outlier <eric901201@gmail.com>
Signed-off-by: Future-Outlier <eric901201@gmail.com> Co-authored-by: pingsutw <pingsutw@apache.org>
flytekit/clients/friendly.py
Outdated
@@ -1046,3 +1048,33 @@ def get_data(self, flyte_uri: str) -> _data_proxy_pb2.GetDataResponse: | |||
|
|||
resp = self._dataproxy_stub.GetData(req, metadata=self._metadata) | |||
return resp | |||
|
|||
def get_download_deck_signed_url( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think we need to change the function name to get_download_signed_url
and add ArtifactType
to the input args?
cc @wild-endeavor @eapolinario
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah. What if we set the default artifact_type
to ARTIFACT_TYPE_DECK
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I can do that
flytekit/clients/friendly.py
Outdated
self, | ||
node_id: str, | ||
project: str, | ||
domain: str, | ||
name: str, | ||
expires_in: datetime.timedelta = None, | ||
) -> _data_proxy_pb2.CreateDownloadLinkResponse: | ||
""" | ||
This is a new API for flyte and union cluster to get the signed url for the deck artifact. | ||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self, | |
node_id: str, | |
project: str, | |
domain: str, | |
name: str, | |
expires_in: datetime.timedelta = None, | |
) -> _data_proxy_pb2.CreateDownloadLinkResponse: | |
""" | |
This is a new API for flyte and union cluster to get the signed url for the deck artifact. | |
""" | |
self, | |
node_id: str, | |
project: str, | |
domain: str, | |
name: str, | |
artifact_type: _data_proxy_pb2.ArtifactType = ARTIFACT_TYPE_DECK, | |
expires_in: datetime.timedelta = None, | |
) -> _data_proxy_pb2.CreateDownloadLinkResponse: | |
""" | |
Get a signed url for an artifact. | |
:param node_id: Node id associated with artifact | |
:param project: Name of the project the resource belongs to | |
:param domain: Name of the domain the resource belongs to | |
:param name: User or system provided value for the resource | |
:param artifact_type: ArtifactType of the artifact requested | |
:param expires_in: If provided this defines a requested expiration duration for the generated url | |
:rtype: flyteidl.service.dataproxy_pb2.CreateDownloadLinkResponse | |
""" |
Signed-off-by: Future-Outlier <eric901201@gmail.com> Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Update, please review it for the final check, thank you all! |
Why are the changes needed?
This is a new API for Flyte to get the signed URL for the deck artifact.
What changes were proposed in this pull request?
add a method called
get_download_deck_signed_url
using the IDLCreateDownloadLinkRequest
andCreateDownloadLinkResponse
.How was this patch tested?
Screenshots
Check all the applicable boxes
Related PRs
Docs link