refactor: deprecate old s3 and use use s3_v2 as default #17963
Conversation
Signed-off-by: tabversion <[email protected]>
That's what I am thinking about. Some old user may follow their experience to proceed with |
Signed-off-by: tabVersion <[email protected]>
Signed-off-by: tabVersion <[email protected]>
| if connector == S3_CONNECTOR { | ||
| // S3 connector is deprecated, use OPENDAL_S3_CONNECTOR instead | ||
| // do s3 -> s3_v2 migration | ||
| let entry = props.get_mut(UPSTREAM_SOURCE_KEY).unwrap(); | ||
| *entry = OPENDAL_S3_CONNECTOR.to_string(); | ||
| connector = OPENDAL_S3_CONNECTOR.to_string(); | ||
| } |
There was a problem hiding this comment.
Will these lines work for an existing job?
I don't think it's necessary to help users write their SQL from connector='s3_v2' to 's3'. While on the other hand, it is necessary to do this for an existing job persisted in Meta.
There was a problem hiding this comment.
Only for new jobs. The existing jobs do not go through frontend again.
There was a problem hiding this comment.
xxchan's proposal #17963 (review) sounds the most reasonable to me, that is, rejecting the s3_v2 in frontend, but allow the existing ones. Can we do that?
There was a problem hiding this comment.
BTW, I think the changes here are rewriting s3 -> s3_v2, which is necessary.
There was a problem hiding this comment.
rejecting the s3_v2 in frontend, but allow the existing ones. Can we do that?
Technically, yes.
My concern is from the product side. Rejecting a higher version but accepting a lower one seems strange. And what if we evolve a new s3 source, we'd call it s3_v2 or s3_v3?
There was a problem hiding this comment.
Leaving s3_v2 as an alias sounds acceptable to me, but please make sure all the docs & test cases are updated to s3.
|
adjust the behavior to ban s3_v2 |
Signed-off-by: tabVersion <[email protected]>
| if connector == S3_CONNECTOR { | ||
| // S3 connector is deprecated, use OPENDAL_S3_CONNECTOR instead | ||
| // do s3 -> s3_v2 migration | ||
| let entry = props.get_mut(UPSTREAM_SOURCE_KEY).unwrap(); | ||
| *entry = OPENDAL_S3_CONNECTOR.to_string(); | ||
| connector = OPENDAL_S3_CONNECTOR.to_string(); | ||
| } |
There was a problem hiding this comment.
Leaving s3_v2 as an alias sounds acceptable to me, but please make sure all the docs & test cases are updated to s3.
Signed-off-by: tabVersion <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
as title
resolve #16645
replace
s3withs3_v2in the frontend so we can migrate the default tos3Checklist
./risedev check(or alias,./risedev c)Documentation
Release note
If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.
please remove the prev s3 source part and rename s3_v2 into s3