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.

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

Cleanup: Remove cassandra queries to the user_keys_hash table #2902

Merged
merged 4 commits into from
Dec 14, 2022

Conversation

jschaul
Copy link
Member

@jschaul jschaul commented Dec 5, 2022

As they are never read anymore since 'onboarding' / auto-connect was removed in #1005. See here for old code with past usage.

Checklist

  • Add a new entry in an appropriate subdirectory of changelog.d
  • Read and follow the PR guidelines

@jschaul jschaul temporarily deployed to cachix December 5, 2022 17:02 Inactive
@jschaul jschaul temporarily deployed to cachix December 5, 2022 17:02 Inactive
@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Dec 5, 2022
@jschaul jschaul temporarily deployed to cachix December 5, 2022 17:09 Inactive
@jschaul jschaul temporarily deployed to cachix December 5, 2022 17:09 Inactive
@jschaul jschaul temporarily deployed to cachix December 5, 2022 17:15 Inactive
@jschaul jschaul temporarily deployed to cachix December 5, 2022 17:15 Inactive
@jschaul jschaul changed the title Cleanup: Remove cassandra queries to the user_keys_hashed table Cleanup: Remove cassandra queries to the user_keys_hash table Dec 5, 2022
@jschaul jschaul requested a review from fisx December 5, 2022 17:16
jschaul added a commit that referenced this pull request Dec 6, 2022
Not needed anymore, see also #2902
@jschaul jschaul temporarily deployed to cachix December 6, 2022 09:15 Inactive
@jschaul jschaul temporarily deployed to cachix December 6, 2022 09:15 Inactive
@jschaul jschaul temporarily deployed to cachix December 6, 2022 16:04 Inactive
@jschaul jschaul temporarily deployed to cachix December 6, 2022 16:04 Inactive
they are never read anymore since 'onboarding' / auto-connect was removed in #1005
@jschaul jschaul temporarily deployed to cachix December 13, 2022 11:56 — with GitHub Actions Inactive
@jschaul jschaul temporarily deployed to cachix December 13, 2022 11:57 — with GitHub Actions Inactive
@@ -157,8 +157,6 @@ runFullScans env@Env {..} users = do
.| mapC (filter (haveId . view _2))

-- FUTUREWORK: no need to read this table, it can be populated from `brig.user`
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe remove this comment as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

👍🏿

@jschaul jschaul temporarily deployed to cachix December 13, 2022 18:26 — with GitHub Actions Inactive
@jschaul jschaul temporarily deployed to cachix December 13, 2022 18:26 — with GitHub Actions Inactive
@mdimjasevic
Copy link
Contributor

The PR didn't run through the PR pipeline in CI. Is that a problem for this PR?

Copy link
Contributor

@mdimjasevic mdimjasevic left a comment

Choose a reason for hiding this comment

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

Is this to be reviewed only after July 2023?

@jschaul
Copy link
Member Author

jschaul commented Dec 14, 2022

The PR didn't run through the PR pipeline in CI. Is that a problem for this PR?

it ran in the past but had a flaky integration test. I rebased and pushed... and CI doesn't seem to pick it up anymore.

@jschaul
Copy link
Member Author

jschaul commented Dec 14, 2022

Is this to be reviewed only after July 2023?

no, this should be merged now, and reviewed now, only the cassandra schema change added after july 2023.

@jschaul jschaul temporarily deployed to cachix December 14, 2022 16:31 — with GitHub Actions Inactive
@jschaul jschaul temporarily deployed to cachix December 14, 2022 16:31 — with GitHub Actions Inactive
@jschaul
Copy link
Member Author

jschaul commented Dec 14, 2022

CI ran after another commit.

@jschaul jschaul merged commit ca215e3 into develop Dec 14, 2022
@jschaul jschaul deleted the remove-hashed-key-queries branch December 14, 2022 17:22
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.

None yet

4 participants