Introduce rawDataBucket #4759
Introduce rawDataBucket #4759
Conversation
dosubot
bot
added
size:L
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4759 +/- ##
==========================================
- Coverage 58.17% 58.08% -0.09%
==========================================
Files 626 367 -259
Lines 53833 32625 -21208
==========================================
- Hits 31315 18949 -12366
+ Misses 20010 12105 -7905
+ Partials 2508 1571 -937
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
charts/flyte-core/values-eks.yaml
Outdated
| @@ -5,6 +5,7 @@ userSettings: | |||
| dbPassword: <DB_PASSWORD> | |||
| rdsHost: <RDS_HOST> | |||
| bucketName: <BUCKET_NAME> | |||
| userBucketName: <RAW_DATA_BUCKET_NAME> | |||
There was a problem hiding this comment.
would it be better to call it rawDataBucketName?
| @@ -179,6 +179,8 @@ data: | |||
| signedUrls: | |||
| durationMinutes: 3 | |||
| storage.yaml: | | |||
| propeller: | |||
| rawoutput-prefix: s3://<RAW_DATA_BUCKET_NAME>/data | |||
There was a problem hiding this comment.
Not needed, it's the way binary handles it currently. I thought it'd be nice to align them
| @@ -498,6 +500,8 @@ data: | |||
| profiler-port: 10254 | |||
| storage-prefix: metadata/datacatalog | |||
| storage.yaml: | | |||
| propeller: | |||
| rawoutput-prefix: | |||
There was a problem hiding this comment.
IIRC, rawoutput-prefix can't be empty. does it work in the sandbox?
There was a problem hiding this comment.
I tested on an EKS env and it works fine but not in sandbox yet.
I'll be testing on sandbox, I think I know why it's rendered as empty here (you're right, it cannot be empty)
davidmirror-ops
force-pushed
the
4291-separate-buckets
branch
from
debccf7 to
fd42f65
Compare
dosubot
bot
added
size:XS
Tracking issue
#4291
Why are the changes needed?
Enables users to specify a separate (from metadata) Raw Data bucket in the form of a userDataBucket parameter in flyte-core.
What changes were proposed in this pull request?
How was this patch tested?
Setup process
Screenshots
Check all the applicable boxes
Related PRs
Docs link