changefeedccl: warn when using https sink urls #107572
Conversation
Fixes cockroachdb#98719. Release note: None
|
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
|
This change is |
HonoreDB
changed the title
| return err | ||
| } | ||
|
|
||
| if u.Scheme == changefeedbase.SinkSchemeCloudStorageHTTP || u.Scheme == changefeedbase.SinkSchemeCloudStorageHTTPS { |
There was a problem hiding this comment.
Do we want to do that?
Are we going to bother people who want http with needles warning?
Would it be better to add file-http(s) scheme and prepare http(s) scheme for deprecation?
@amruss ?
|
|
||
| if u.Scheme == changefeedbase.DeprecatedSinkSchemeHTTP || u.Scheme == changefeedbase.DeprecatedSinkSchemeHTTP { | ||
| p.BufferClientNotice(ctx, pgnotice.Newf( | ||
| `%s sinks will emit using cloud storage semantics. For a webhook sink, prepend webhook- to the sink URI. Prepend file- to remove this notice.`, |
There was a problem hiding this comment.
maybe rephrase a bit to avoid using "prepend"? Something like "Use webhook-https for webhook semantics, use file-http:// .."
HonoreDB
force-pushed
the
warn_on_ambiguous_http_sink
branch
2 times, most recently
from
c68daea
to
981979e
Compare
Release note's in the second commit, merged it into the PR description. |
Changefeeds previously used http as their URI to align with backups. Backups no longer support http, and for changefeeds http is ambiguous, leading users to create cloudstorage sinks when they meant webhook sinks. This commit changes the scheme to file-http. Webhooks still use webhook-http. Informs: cockroachdb#98719 Release note (enterprise change): Changefeeds that create files over an http connection may now be specified via `INTO 'file-https://'` to disambiguate with `webhook-https`.
HonoreDB
force-pushed
the
warn_on_ambiguous_http_sink
branch
from
981979e
to
a428c45
Compare
|
bors r=[miretskiy] |
|
Build failed (retrying...): |
|
Build succeeded: |
Changefeeds previously used http as their URI to align with backups.
Backups no longer support http, and for changefeeds http is ambiguous,
leading users to create cloudstorage sinks when they meant webhook sinks.
This PR changes the scheme to file-http. Webhooks still use webhook-http.
Fixes #98719.
Release note (enterprise change): Changefeeds that create files over an http connection may now be specified via
INTO 'file-https://'to disambiguate withwebhook-https.