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

WPB-4748 create a way to inspect background notifications queue #3589

Merged
merged 20 commits into from
Oct 17, 2023
Merged

WPB-4748 create a way to inspect background notifications queue #3589

merged 20 commits into from
Oct 17, 2023

Conversation

battermann
Copy link
Contributor

@battermann battermann commented Sep 19, 2023 •
edited
Loading

https://wearezeta.atlassian.net/browse/WPB-4748

Checklist

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

@zebot zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Sep 19, 2023
@battermann battermann force-pushed the WPB-4748-create-a-way-to-inspect-background-notifications-queue branch from 09fa9c6 to 571b37c Compare September 19, 2023 15:20
@battermann battermann force-pushed the WPB-4748-create-a-way-to-inspect-background-notifications-queue branch from 114590e to 480429c Compare September 26, 2023 13:08
@battermann battermann force-pushed the WPB-4748-create-a-way-to-inspect-background-notifications-queue branch from 480429c to 6d28fb2 Compare September 26, 2023 13:12
@battermann battermann marked this pull request as ready for review September 28, 2023 11:00
akshaymankar
akshaymankar reviewed Oct 2, 2023
View reviewed changes
Copy link
Member

@akshaymankar akshaymankar left a comment

Choose a reason for hiding this comment

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

Looks good overall, some minor comments inline.
Apart from those, could you please add this to the executablesMap in nix/wire-server.nix so we keep building the docker image for this?

wire-server/nix/wire-server.nix

Lines 73 to 87 in 5f246fb

executablesMap = {
brig = [ "brig" "brig-index" "brig-integration" "brig-schema" ];
cannon = [ "cannon" ];
cargohold = [ "cargohold" "cargohold-integration" ];
federator = [ "federator" "federator-integration" ];
galley = [ "galley" "galley-integration" "galley-schema" "galley-migrate-data" ];
gundeck = [ "gundeck" "gundeck-integration" "gundeck-schema" ];
proxy = [ "proxy" ];
spar = [ "spar" "spar-integration" "spar-schema" "spar-migrate-data" ];
stern = [ "stern" "stern-integration" ];
inconsistencies = [ "inconsistencies" ];
zauth = [ "zauth" ];
background-worker = [ "background-worker" ];
integration = [ "integration" ];
};

tools/rabbitmq-consumer/src/RabbitMQConsumer/Lib.hs Outdated
Comment on lines 35 to 40
runTimerAsync done opts.timeoutSec
case opts.cmd of
Head -> void $ consumeMsgs chan opts.queue Ack (printHead done opts)
Interactive -> void $ consumeMsgs chan opts.queue Ack (interactive done opts)
DropHead dhOpts -> void $ consumeMsgs chan opts.queue Ack (dropHead done opts dhOpts)
takeMVar done
Copy link
Member

Choose a reason for hiding this comment

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

Seems wrong to run timer like this for interactive session. Things could timeout when the user is just looking at the JSON.

tools/rabbitmq-consumer/src/RabbitMQConsumer/Lib.hs Outdated
[ "vhost: " <> cs opts.vhost,
"queue: " <> cs opts.queue,
"timestamp: " <> show msg.msgTimestamp,
"received message: " <> BL.unpack msg.msgBody
Copy link
Member

Choose a reason for hiding this comment

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

We should try to decode the body as a Value and pretty print the json.

akshaymankar
akshaymankar reviewed Oct 16, 2023
View reviewed changes
cabal.project Outdated Show resolved Hide resolved
@akshaymankar
Copy link
Member

Error in CI

FATA[0001] trying to reuse blob sha256:4a660d37c91f870fb044f7ac64fd807cd5990d298288183e9b97cbbb228c5d3f at destination: checking whether a blob sha256:4a660d37c91f870fb044f7ac64fd807cd5990d298288183e9b97cbbb228c5d3f exists in quay.io/wire/rabbitmq-consumer: authentication required

Probably means that the new image needs to be made publically readable and also the bot needs rights to be able to push it.

elland
elland reviewed Oct 17, 2023
View reviewed changes
tools/rabbitmq-consumer/rabbitmq-consumer.cabal
, wire-api
, wire-api-federation

default-extensions:
Copy link
Contributor

Choose a reason for hiding this comment

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

At some point we should go around cleaning up these extensions since a lot of them are on by default with GHC2021, right?

akshaymankar
akshaymankar approved these changes Oct 17, 2023
View reviewed changes
Copy link
Member

@akshaymankar akshaymankar left a comment

Choose a reason for hiding this comment

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

🚀

@battermann battermann merged commit d18bbd2 into develop Oct 17, 2023
9 checks passed
@battermann battermann deleted the WPB-4748-create-a-way-to-inspect-background-notifications-queue branch October 17, 2023 10:09
This was referenced Oct 19, 2023
Closed
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elland elland elland left review comments

@akshaymankar akshaymankar akshaymankar approved these changes

Assignees
No one assigned
Labels
ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@battermann @akshaymankar @elland @zebot