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

Update storage config in value-gcp.yaml #5003

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Update storage config in value-gcp.yaml #5003

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
16 changes: 9 additions & 7 deletions charts/flyte-core/values-gcp.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -194,14 +194,16 @@ common:
#

storage:
# -- Sets the storage type. Supported values are sandbox, s3, gcs and custom.
type: gcs
Copy link
Member Author

Choose a reason for hiding this comment

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

@katrogan / @davidmirror-ops do you know how does it work before? we don't support gsc type.

flyte/flytestdlib/storage/config.go

Lines 47 to 65 in 74b0a1b

type Config struct {
Type Type `json:"type" pflag:",Sets the type of storage to configure [s3/minio/local/mem/stow]."`
// Deprecated: Please use StowConfig instead
Connection ConnectionConfig `json:"connection"`
Stow StowConfig `json:"stow,omitempty" pflag:",Storage config for stow backend."`
// Container here is misleading, it refers to a Bucket (AWS S3) like blobstore entity. In some terms it could be a table
InitContainer string `json:"container" pflag:",Initial container (in s3 a bucket) to create -if it doesn't exist-.'"`
// By default, if this is not enabled, multiple containers are not supported by the storage layer. Only the configured `container` InitContainer will be allowed to requests data from. But, if enabled then data will be loaded to written to any
// container specified in the DataReference.
MultiContainerEnabled bool `json:"enable-multicontainer" pflag:",If this is true, then the container argument is overlooked and redundant. This config will automatically open new connections to new containers/buckets as they are encountered"`
// Caching is recommended to improve the performance of underlying systems. It caches the metadata and resolving
// inputs is accelerated. The size of the cache is large so understand how to configure the cache.
// TODO provide some default config choices
// If this section is skipped, Caching is disabled
Cache CachingConfig `json:"cache"`
Limits LimitsConfig `json:"limits" pflag:",Sets limits for stores."`
DefaultHTTPClient HTTPClientConfig `json:"defaultHttpClient" pflag:",Sets the default http client config."`
SignedURL SignedURLConfig `json:"signedUrl" pflag:",Sets config for SignedURL."`

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure! I know for our GCP deployments we've been setting type: stow

Copy link
Contributor

Choose a reason for hiding this comment

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

For the reference implementation, we've been using storage.type: gcs with no issues

Copy link
Contributor

Choose a reason for hiding this comment

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

aha @pingsutw we overwrite it here:

flyte/charts/flyte-core/templates/_helpers.tpl

Line 228 in f9c1a97

type: stow

Copy link
Contributor

Choose a reason for hiding this comment

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

Any idea about the difference between minio and local? We've been using sandbox (rewritten as minio) for on-prem deployments with flyte-core. Maybe local makes more sense.

# -- Sets the storage type. Supported values are s3/minio/local/mem/stow.
type: stow
stow:
kind: google
config:
json: ""
project_id: "{{ .Values.userSettings.googleProjectId }}"
scopes: https://www.googleapis.com/auth/cloud-platform
# -- bucketName defines the storage bucket flyte will use. Required for all types except for sandbox.
bucketName: "{{ .Values.userSettings.bucketName }}"
# -- settings for storage type s3
gcs:
# -- GCP project ID. Required for storage type gcs.
projectId: "{{ .Values.userSettings.googleProjectId }}"
container: "{{ .Values.userSettings.bucketName }}"

db:
datacatalog:
Expand Down
2 changes: 1 addition & 1 deletion flytestdlib/storage/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ type Config struct {

// Container here is misleading, it refers to a Bucket (AWS S3) like blobstore entity. In some terms it could be a table
InitContainer string `json:"container" pflag:",Initial container (in s3 a bucket) to create -if it doesn't exist-.'"`
// By default if this is not enabled, multiple containers are not supported by the storage layer. Only the configured `container` InitContainer will be allowed to requests data from. But, if enabled then data will be loaded to written to any
// By default, if this is not enabled, multiple containers are not supported by the storage layer. Only the configured `container` InitContainer will be allowed to requests data from. But, if enabled then data will be loaded to written to any
// container specified in the DataReference.
MultiContainerEnabled bool `json:"enable-multicontainer" pflag:",If this is true, then the container argument is overlooked and redundant. This config will automatically open new connections to new containers/buckets as they are encountered"`
// Caching is recommended to improve the performance of underlying systems. It caches the metadata and resolving
Expand Down
Loading