Skip to content
This repository has been archived by the owner on Oct 23, 2023. It is now read-only.

Support for containers without Flytekit #62

Merged
merged 4 commits into from
May 12, 2020
Merged

Conversation

kumare3
Copy link
Contributor

@kumare3 kumare3 commented May 12, 2020

TL;DR

New Plugin for support Flyte CoPilot and arbitrary containers without flytekit

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code documentation added
  • Any pending items have an associated Issue

Complete description

Design proposal: https://docs.google.com/document/d/14H5UESK91YN6s1wMGY9EFh-FO8nCOQkJrieK3NQqV08/edit#

Tracking Issue

flyteorg/flyte#297

Follow-up issue

NA

@kumare3 kumare3 requested review from katrogan and EngHabu May 12, 2020 19:14
// it is recommended to use the PROTO format.
// JSON and YAML do not need any protobuf definitions to read it
enum MetadataFormat {
JSON = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

why is JSON the defaultr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because, it is assumed that users of raw container plugin do not have flytekit and hence cannot easily read protobufs

}
// File system path (start at root). This folder will contain all the inputs exploded to a separate file.
// Example, if the input interface needs (x: int, y: blob, z: multipart_blob) and the input path is "/var/flyte/inputs", then the file system will look like
// /var/flyte/inputs/inputs.<metadata format dependent -> .pb .json .yaml>
Copy link
Contributor

Choose a reason for hiding this comment

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

what does this file contain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it contains a version of the LiteralMap, but I will clarify in the documentation and update it here

Copy link
Contributor

Choose a reason for hiding this comment

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

thank you!

katrogan
katrogan previously approved these changes May 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants