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

Implement AMQP Message client for Job updates #155

Open
psiemens opened this issue Aug 25, 2021 · 3 comments
Open

Implement AMQP Message client for Job updates #155

psiemens opened this issue Aug 25, 2021 · 3 comments
Labels
enhancement New feature or request future features

Comments

@psiemens
Copy link
Collaborator

This issue was originally opened here by @davidhouseholter-hah on August 20, 2021: onflow/wallet-api#17

I added a new package to publish messages to a message queue. I still need to add the event bus name to the cfg file but I thought it was best to talk to members of the team about architecture. I would like this logic to use the JobService but currently, the service is not being used for creating or update.

The changes I have made currently: https://github.com/onflow/wallet-api/compare/main...davidhouseholter-hah:ampq_message_client?expand=1

@psiemens
Copy link
Collaborator Author

@latenssi @nanuuki @tehranifar Do you have any input on the architecture here?

@nvdtf
Copy link
Collaborator

nvdtf commented Aug 25, 2021

My quick feedback:

  • There needs to be a layer of abstraction between the job service and AMQP package, because we might need to support other message queues in the future, like Kafka.
  • Some functions like UpdateJob might need to be renamed to better accommodate the new "store or publish" logic. Going a step further I don't think publishing should be done in the database layer, so a rewrite is needed.
  • Needs unit tests + docker compose integration with a sample AMQP service for local development.
  • The new env variables need to be renamed to specify AMQP in their names.
  • AMQP publish should be disabled by default and only in use when configured.
  • Seems to me there are a lot of config variables in the AMQP service. We might need to expose some of them in the env config.

@nanuuki nanuuki changed the title Implement AMPQ Message client for Job updates Implement AMQP Message client for Job updates Aug 26, 2021
@sheerryy
Copy link

sheerryy commented Sep 1, 2021

AMQP Message client for Job updates is very hand. Can you guys give any ETA for this feature? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request future features
Projects
None yet
Development

No branches or pull requests

4 participants