Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # 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!

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

No branches or pull requests

4 participants