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

Kafka events #38

Merged
merged 230 commits into from
Dec 1, 2023
Merged

Kafka events #38

merged 230 commits into from
Dec 1, 2023

Conversation

dbluhm
Copy link
Contributor

@dbluhm dbluhm commented Nov 15, 2023

This PR adds the kafka-events plugin from https://github.com/sicpa-dlab/aries-acapy-plugin-kafka-events.

I open this PR on behalf of the SICPA team who have generously agreed to contribute this plugin both to HL and to the plugins repo.

Thank you SICPA team!

This PR includes the history of the plugin. I find this valuable but I'm open to thoughts on this approach.

victormartinez-work and others added 30 commits April 26, 2021 16:33
Signed-off-by: Adam Burdett <[email protected]>
Signed-off-by: Adam Burdett <[email protected]>
Signed-off-by: Adam Burdett <[email protected]>
Signed-off-by: Adam Burdett <[email protected]>
- docker compose with zookeeper/kafka image from an example.
- pytest setup/teardown and empty simple test for kafka.

Signed-off-by: Adam Burdett <[email protected]>
Signed-off-by: Luis Gomez <[email protected]>
Signed-off-by: Luis Gomez <[email protected]>
Signed-off-by: Luis Gomez <[email protected]>
Signed-off-by: Luis Gomez <[email protected]>
Kafka consumer & producer with lib aiokafka
Signed-off-by: Luis Gomez <[email protected]>
Signed-off-by: Luis Gomez <[email protected]>
Rest client to consume and produce records to kafka
@swcurran
Copy link
Contributor

@jamshale can you please do the “mechanical” review of the PR? Are all the pieces needed by the plugins guidelines to be added to the repo.

@dbluhm — I see that the DCO is not complete. Thoughts on what to do about that?

@dbluhm
Copy link
Contributor Author

dbluhm commented Nov 15, 2023

Indeed, I'll work with the SICPA to get those commits corrected. On the mechanical points, there are some small tweaks to make (renaming a folder, adding ruff, etc.)

@jamshale
Copy link
Contributor

This looks good. It seems very similar to the conversion I did from aries-acapy-plugin-redis-events. I would like it to change structure to follow what redis_events is like if possible. The integration tests folder will need to be renamed for the tests to run correctly and I think the workflow files could be removed. There's definitely a bit of work to make it consistent with the other ones.

I've just done almost the same thing so I could do this a well if we wanted, but I wouldn't get it done before I go on vacation.

It is really handy to have the commit history. It would have helped show what I did with the redis_events plugin. I wasn't sure how I could do that.

@jamshale
Copy link
Contributor

I think I'll try and update this to try and get the unit, linting, and integration tests working. Then I'll create another ticket to adjust the structure as another task.

@dbluhm
Copy link
Contributor Author

dbluhm commented Nov 17, 2023

@jamshale quick warning that the DCO issues will require a history rewrite to correct. I think you should be able to just rebase any changes you make but wise to keep that in mind 😅 As I already mentioned, I'll work with the SICPA team on getting the commits missing a sign-off taken care of but your contributions on those things (unit and int testing, etc) would be greatly appreciated!

@jamshale
Copy link
Contributor

I changed the structure of the plugin and adjusted a few files. I added docs for ruff linting and changed the linting/testing configs. The integration tests and unit tests and linting checks are now working. I removed some files related to github actions and pre commit linting checks. Some of these would be nice to have but at a repo level.

I think this could be merged now when the DCO check is fixed. There are several things I would still like to change/update before this plugin is considered finished.

  • Dependencies in general and acapy dependency specifically should be updated to the level of the other plugins.
  • It'd be nice to have the Dockerfiles updated to be consistent with the existing plugins.
  • Some documentation needs to be updated still after changing the structure.
  • Would like more unit testing in general. Hopefully the integration tests catch things, but I've already seen unit tests be beneficial in catching unknown breaking changes in acapy.

I'll see if I can get anymore done today. Otherwise we can merge it as is and I can finish these things when I'm back from vacation.

@dbluhm
Copy link
Contributor Author

dbluhm commented Nov 28, 2023

@ryjones seeking advice on the best way to handle the DCO issues on this PR. I am in contact with the contributors of the affected commits and they are willing to consider those commits "signed off" according to the DCO. The authors are not, however, in an ideal position to rewrite the history of this branch to add those sign-off messages. Is this a case where using third party remediation would be appropriate? https://github.com/dcoapp/app#third-party-remediation-support

@ryjones
Copy link
Contributor

ryjones commented Nov 28, 2023

@dbluhm these are the commits that need blessed. If each person could put a commit at the tip of the branch saying "I sign off these commits" with the hashes, that would work.

Adam Burdett <[email protected]>
662e5c6935d7cd6b4faaec313f98d1a087fd8ec1
35996591bded86de0fd1851c535604fb434f262e
66a26665c7a419a430cfbe39a11813f54a2befd5
71e9444b9ab3f1e6ed8ee5c63c2b74a9a5a85480
937f341ee21dc034808b1f949ad08991c023fdc4
f4f8cf368ab6daa2c15f3166502024504f7ae465
1b7c2320f043dcbd8749ad8df983524ac37725b9

Clément Humbert <[email protected]>
6ea3f965ba8e2a1cc34ae1de96d17471c002fd9e
524b4dab79088d72da089600a512b33a01552361
17e4c471e5c9152ee57d3f81af08bec23b680130
faa65d83cc1d4e0d3281ff3e84afa63e885af094
dd14ba52279636853b0a8d5c83e76b52d54ad907
e15bf96df12d75fb402fc565be2090d879f5099a

Daniel Bluhm <[email protected]>
db0429131789e2b929057c4fe84981ae325e2d14
4a694f40ac348ba06a1f2900a9ae4820c7119778
c5eca2070b229a0d3dfc4d5f8fa201533e69bc28
f6e3c8ffb588f199eddfd03bf53a15dea5a123b0
2b665eaa53c0d14f9f5e23cdab860ba114cd6a42
2f4a5d1481a60477f07e04eb559e43e39b05c808
58652d7f9626fc3b0714a6fce4a1c735aeca368f
f26780f011fdf3ffe2f9f1f90d84d9590a7eca26
ee5d7ee8acd987db4a5ef1f57b154ea8cea517a5
43c785c4f133bfc12430f5ba45b66ee278a97489
584e0bce6957039d0d675f9dceb50e629086b33c
fd26fd5e6e2efd058067db121fc8ef63b510f983
2458cf4a73ad14fbc2137473ca6526b28df41ab5
5b135fa0b86abbfbeffc69329f04e2493b934d5d
d694e5371c82f8125cf4a0ff39a74e2f695fa8fa
3603f1212610a126a6bca68fc0df6caccc98efde
aed188f38d45053fa35cf61c313426235db04f6f
2d9f0c806fca8667bbab95b920a14010d9e48020
f5dfdd02b57fc562dc9493bacbd5a1b8008bdf0f
34eed302116a38acbd2d26776f001eb8e8892fb5
3011c082a8e4c643c7cab70b6a0e48b78c20dd78
3fc7d96088b2ee86ce35fa8add1af1920782c1d5
d506cfa06e88dfd5e3b8d960c3b5a21bb4bd24be
d343e0c1d7c8eb2aff502a83ce0f852eba27ac12
c2e482803333f3abe9a7f0fe9643851323d6d5e8
8d87630dfdd40a0bf913b6c420dc7d6bb2db6a28
72f0b8fe65a5266324408debfdb14e2cb663ce5c
da5867a01157180c2a3824b07852ef3a59b9f128
99369a977d0ff997cea1d6db5da54576025dee90
ba8f5d8c5635fc0263a12f8a13cee2732524669d

Darko Kulic <[email protected]>
e98ffb60b097fb89a991712911a498937b1ff01d
09dcf37f7e69d93a957854f63c45bec32ee159ee

Luis Gomez <[email protected]>
082937d17509387e7e969511f25c4976c7137744
e006b61481ba76dc18ba23bcb160a61adde0e445

Matteo Marangoni <[email protected]>
0fb7400a9692b03b2e6dadaa62931c1a418ff909
becea1dee0c39dd12d257fec29420e19823ed1cc

Nemanja Patrnogic <[email protected]>
4d2028f0f6bef586fcdf1d2f43900a2e83474237
2df89a29e5e37d11a0985d055c5be4501030434a
44d23d8f9038966201ca3315638274d49a437044
44b2be9d006a8a988372ca874c9397b07e758827
91e7271c779974bd075ecea3605593fbbcc0fc77

Víctor Martínez Jurado <[email protected]>
a8c325369be6329475e49e5dc38872acae4ad0c8

@dbluhm
Copy link
Contributor Author

dbluhm commented Nov 29, 2023

@ryjones, unfortunately, Luis Gomez is not with SICPA anymore. Can @chumbert sign off the commits on his behalf?

@ryjones
Copy link
Contributor

ryjones commented Nov 29, 2023

@dbluhm yes - in fact, one representative from SICPA can sign them all off.

dbluhm and others added 2 commits November 29, 2023 10:15
I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: e15bf96
I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: dd14ba5
I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: faa65d8
I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: 17e4c47
I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: 524b4da

Third-Party DCO Remediation Commit for Darko Kulic <[email protected]>

On behalf of Darko Kulic <[email protected]>, I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: 09dcf37

Third-Party DCO Remediation Commit for Luis Gomez <[email protected]>

On behalf of Luis Gomez <[email protected]>, I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: e006b61

Third-Party DCO Remediation Commit for Matteo Marangoni <[email protected]>

On behalf of Matteo Marangoni <[email protected]>, I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: becea1d

Third-Party DCO Remediation Commit for Nemanja Patrnogic <[email protected]>

On behalf of Nemanja Patrnogic <[email protected]>, I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: 91e7271
On behalf of Nemanja Patrnogic <[email protected]>, I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: 44b2be9
On behalf of Nemanja Patrnogic <[email protected]>, I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: 44d23d8

Third-Party DCO Remediation Commit for Víctor Martínez Jurado <[email protected]>

On behalf of Víctor Martínez Jurado <[email protected]>, I, Clément Humbert <[email protected]>, hereby add my Signed-off-by to this commit: a8c3253

Signed-off-by: Clément Humbert <[email protected]>
@dbluhm
Copy link
Contributor Author

dbluhm commented Dec 1, 2023

@jamshale @swcurran This is ready for final review!

Copy link
Contributor

@jamshale jamshale left a comment

Choose a reason for hiding this comment

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

Will change things a bit more to be consistent with redis_events and add some unit test coverage in future PR. Will merge this as is for now. Thanks so much SCIPA team!

@swcurran swcurran merged commit f07e9d6 into main Dec 1, 2023
3 checks passed
@jamshale jamshale deleted the kafka-events branch July 22, 2024 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.