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

Jump to bottom

Unify --allow-unsupported-set and --unsupported-set-mode #1354

Closed
altmannmarcelo opened this issue Aug 20, 2024 · 0 comments
Closed

Unify --allow-unsupported-set and --unsupported-set-mode #1354

altmannmarcelo opened this issue Aug 20, 2024 · 0 comments
Assignees
@jasobrown-rs
Labels
1 points Created by Linear-GitHub Sync first-issue Created by Linear-GitHub Sync Low priority Created by Linear-GitHub Sync
Milestone
v.45

Comments

@altmannmarcelo
Copy link
Contributor

Description

Today we have --unsupported-set-mode that allows for two values, Error and Proxy. but we also have --allow-unsupported-set as a dedicated variable. Those two variables all control what we set in:

pub enum UnsupportedSetMode {
    /// Return an error to the client (the default)
    Error,
    /// Proxy all subsequent statements to the upstream
    Proxy,
    /// Allow all unsupported set statements
    Allow,
}

We should unify them by adding allow as one option to --unsupported-set-mode

Change in user-visible behavior

Requires documentation change
@altmannmarcelo altmannmarcelo added Low priority Created by Linear-GitHub Sync first-issue Created by Linear-GitHub Sync labels Aug 29, 2024
@jasobrown-rs jasobrown-rs mentioned this issue Sep 25, 2024
Open
readysetbot pushed a commit that referenced this issue Sep 25, 2024
@jasobrown-rs
adapter: Merge --allow-unsupported-set
5d8d00d 
`--allow-unsupported-set` was added in
c1df0d7, and it allowed readyset is
naively ignore any `SET` commands it could nor process.

About a year later, in e86017f, we
introduced the `--unsupported-set-mode` flag and enum. One of the
modes available is `Allow`, that mapped directly onto the existing
`--allow-unsupported-set`.

Confusingly, we did not remove the older flag, and we did not allow
setting `--unsupported-set-mode allow`.

Let's reduce our cli flag burden and just merge the two behaviors,
into the way it looks like it was intended to be.

Fixes: REA-4673
Fixes: #1354
Release-Note-Core: Remove hidden `--allow-unsupported-set` flag,
  replaced by `--unsupported-set-mode allow`.
Change-Id: I368ca7ee583bb64d3e44e22b81576c79551d62ff
readysetbot pushed a commit that referenced this issue Sep 25, 2024
@jasobrown-rs
adapter: Merge --allow-unsupported-set
8ea66df 
`--allow-unsupported-set` was added in
c1df0d7, and it allowed readyset to
naively ignore any `SET` commands it could not process.

About a year later, in e86017f, we
introduced the `--unsupported-set-mode` flag and enum. One of the
modes available is `Allow`, that mapped directly onto the existing
`--allow-unsupported-set`.

Confusingly, we did not remove the older flag, and we did not allow
setting `--unsupported-set-mode allow`.

Let's reduce our cli flag burden and just merge the two behaviors,
into the way it looks like it was intended to be.

Fixes: REA-4673
Fixes: #1354
Release-Note-Core: Remove hidden `--allow-unsupported-set` flag,
  replaced by `--unsupported-set-mode allow`.
Change-Id: I368ca7ee583bb64d3e44e22b81576c79551d62ff
@altmannmarcelo altmannmarcelo modified the milestones: v.44, v.45 Sep 26, 2024
readysetbot pushed a commit that referenced this issue Sep 30, 2024
@jasobrown-rs
adapter: Merge --allow-unsupported-set
4f16cdb 
`--allow-unsupported-set` was added in
c1df0d7, and it allowed readyset to
naively ignore any `SET` commands it could not process.

About a year later, in e86017f, we
introduced the `--unsupported-set-mode` flag and enum. One of the
modes available is `Allow`, that mapped directly onto the existing
`--allow-unsupported-set`.

Confusingly, we did not remove the older flag, and we did not allow
setting `--unsupported-set-mode allow`.

Let's reduce our cli flag burden and just merge the two behaviors,
into the way it looks like it was intended to be.

Fixes: REA-4673
Fixes: #1354
Release-Note-Core: Remove hidden `--allow-unsupported-set` flag,
  replaced by `--unsupported-set-mode allow`.
Change-Id: I368ca7ee583bb64d3e44e22b81576c79551d62ff
readysetbot pushed a commit that referenced this issue Sep 30, 2024
@jasobrown-rs
adapter: Merge --allow-unsupported-set
cc70fb8 
`--allow-unsupported-set` was added in
c1df0d7, and it allowed readyset to
naively ignore any `SET` commands it could not process.

About a year later, in e86017f, we
introduced the `--unsupported-set-mode` flag and enum. One of the
modes available is `Allow`, that mapped directly onto the existing
`--allow-unsupported-set`.

Confusingly, we did not remove the older flag, and we did not allow
setting `--unsupported-set-mode allow`.

Let's reduce our cli flag burden and just merge the two behaviors,
into the way it looks like it was intended to be.

Fixes: REA-4673
Fixes: #1354
Release-Note-Core: Remove hidden `--allow-unsupported-set` flag,
  replaced by `--unsupported-set-mode allow`.
Change-Id: I368ca7ee583bb64d3e44e22b81576c79551d62ff
@altmannmarcelo altmannmarcelo added the 1 points Created by Linear-GitHub Sync label Sep 30, 2024
readysetbot pushed a commit that referenced this issue Oct 2, 2024
@jasobrown-rs
adapter: Merge --allow-unsupported-set
1b1ec40 
`--allow-unsupported-set` was added in Ic1df0d73, and it allowed
readyset to naively ignore any `SET` commands it could not process.

About a year later, in Ie86017f8, we introduced the
`--unsupported-set-mode` flag and enum. One of the modes available is
`Allow`, that mapped directly onto the existing `--allow-unsupported-set`.

Confusingly, we did not remove the older flag, and we did not allow
setting `--unsupported-set-mode allow`.

Let's reduce our cli flag burden and just merge the two behaviors,
into the way it looks like it was intended to be.

Fixes: REA-4673
Fixes: #1354
Release-Note-Core: Remove hidden `--allow-unsupported-set` flag,
  replaced by `--unsupported-set-mode allow`.
Change-Id: I368ca7ee583bb64d3e44e22b81576c79551d62ff
readysetbot pushed a commit that referenced this issue Oct 2, 2024
@jasobrown-rs
adapter: Merge --allow-unsupported-set
4c45276 
`--allow-unsupported-set` was added in Ie7bfc9cd, and it allowed
readyset to naively ignore any `SET` commands it could not process.

About a year later, in I0005339c, we introduced the
`--unsupported-set-mode` flag and enum. One of the modes available is
`Allow`, that mapped directly onto the existing `--allow-unsupported-set`.

Confusingly, we did not remove the older flag, and we did not allow
setting `--unsupported-set-mode allow`.

Let's reduce our cli flag burden and just merge the two behaviors,
into the way it looks like it was intended to be.

Fixes: REA-4673
Fixes: #1354
Release-Note-Core: Remove hidden `--allow-unsupported-set` flag,
  replaced by `--unsupported-set-mode allow`.
Change-Id: I368ca7ee583bb64d3e44e22b81576c79551d62ff
readysetbot pushed a commit that referenced this issue Oct 2, 2024
@jasobrown-rs
adapter: Merge --allow-unsupported-set
5f77c51 
`--allow-unsupported-set` was added in Ie7bfc9cd, and it allowed
readyset to naively ignore any `SET` commands it could not process.

About a year later, in I0005339c, we introduced the
`--unsupported-set-mode` flag and enum. One of the modes available is
`Allow`, that mapped directly onto the existing `--allow-unsupported-set`.

Confusingly, we did not remove the older flag, and we did not allow
setting `--unsupported-set-mode allow`.

Let's reduce our cli flag burden and just merge the two behaviors,
into the way it looks like it was intended to be.

Fixes: REA-4673
Fixes: #1354
Release-Note-Core: Remove hidden `--allow-unsupported-set` flag,
  replaced by `--unsupported-set-mode allow`.
Change-Id: I368ca7ee583bb64d3e44e22b81576c79551d62ff
readysetbot pushed a commit that referenced this issue Oct 2, 2024
@jasobrown-rs
adapter: Merge --allow-unsupported-set
821daaa 
`--allow-unsupported-set` was added in Ie7bfc9cd, and it allowed
readyset to naively ignore any `SET` commands it could not process.

About a year later, in I0005339c, we introduced the
`--unsupported-set-mode` flag and enum. One of the modes available is
`Allow`, that mapped directly onto the existing `--allow-unsupported-set`.

Confusingly, we did not remove the older flag, and we did not allow
setting `--unsupported-set-mode allow`.

Let's reduce our cli flag burden and just merge the two behaviors,
into the way it looks like it was intended to be.

Fixes: REA-4673
Fixes: #1354
Release-Note-Core: Remove hidden `--allow-unsupported-set` flag,
  replaced by `--unsupported-set-mode allow`.
Change-Id: I368ca7ee583bb64d3e44e22b81576c79551d62ff
Reviewed-on: https://gerrit.readyset.name/c/readyset/+/8063
Tested-by: Buildkite CI
Reviewed-by: Marcelo Altmann <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jasobrown-rs jasobrown-rs

Labels
1 points Created by Linear-GitHub Sync first-issue Created by Linear-GitHub Sync Low priority Created by Linear-GitHub Sync
Projects
None yet
Milestone
v.45
Development

No branches or pull requests
2 participants
@altmannmarcelo @jasobrown-rs