Skip to content

Fix cql-io bug where restarting C* cluster could cause downtime#2640

Merged
akshaymankar merged 1 commit intodevelopfrom
akshaymankar/fix-cql-io-control-bug
Aug 23, 2022
Merged

Fix cql-io bug where restarting C* cluster could cause downtime#2640
akshaymankar merged 1 commit intodevelopfrom
akshaymankar/fix-cql-io-control-bug

Conversation

@akshaymankar
Copy link
Member

Upstream changes in https://gitlab.com/twittner/cql-io/-/merge_requests/20

JIRA: https://wearezeta.atlassian.net/browse/SQCORE-1345

Checklist

  • The PR Title explains the impact of the change.
  • The PR description provides context as to why the change should occur and what the code contributes to that effect. This could also be a link to a JIRA ticket or a Github issue, if there is one.
  • changelog.d contains the following bits of information (details):
    • A file with the changelog entry in one or more suitable sub-sections. The sub-sections are marked by directories inside changelog.d.

@akshaymankar akshaymankar temporarily deployed to cachix August 22, 2022 08:55 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix August 22, 2022 08:55 Inactive
@akshaymankar
Copy link
Member Author

Please approve this PR only if you approve of the upstream MR.

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Aug 22, 2022
@akshaymankar akshaymankar force-pushed the akshaymankar/fix-cql-io-control-bug branch from a6d8baa to 1ae2f7a Compare August 22, 2022 09:23
@akshaymankar akshaymankar temporarily deployed to cachix August 22, 2022 09:23 Inactive
@akshaymankar akshaymankar temporarily deployed to cachix August 22, 2022 09:23 Inactive
any.hourglass-orphans ==0.1.0.0,
any.hp2pretty ==0.10,
any.hpack ==0.34.5,
any.hpack-dhall ==0.5.3,
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this added here? don't mind either way, just surprised.

Copy link
Member Author

Choose a reason for hiding this comment

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

I just ran the script in tools/convert-to-cabal to keep stack.yaml and cabal.project in sync. I was confused too, but then thought maybe someone added some dependency and forgot to run that script.

Copy link
Member

@jschaul jschaul left a comment

Choose a reason for hiding this comment

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

Is it possible to create a test for this behaviour before/after; or is that not so easy? If not, would it be possible to detail the manual test procedure steps either here in wire-server in some docs; or in the MR upstream on how you tested the change? (internal jira comments can't be read by cql-io maintainers or anyone else interested)

@akshaymankar
Copy link
Member Author

Is it possible to create a test for this behaviour before/after; or is that not so easy? If not, would it be possible to detail the manual test procedure steps either here in wire-server in some docs; or in the MR upstream on how you tested the change? (internal jira comments can't be read by cql-io maintainers or anyone else interested)

I think it is quite hard to create an automated test for this since it requires talking to kubernetes. I wrote about my method of testing in the upstream MR: https://gitlab.com/twittner/cql-io/-/merge_requests/20#note_1074472850

@akshaymankar akshaymankar merged commit a8ba112 into develop Aug 23, 2022
@akshaymankar akshaymankar deleted the akshaymankar/fix-cql-io-control-bug branch August 23, 2022 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants