-
Notifications
You must be signed in to change notification settings - Fork 178
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
[backend] Add Zulip Backend #667
Conversation
The PR is not completed yet and needs a lot of improvements. I am still figuring out how to use offset based method (I have a doubt if it is applicable 🤔). Some of the channels where we can test this are The usage of this backend will be
You can generate the BOT_EMAIL_ADDRESS and BOT_API_KEY from the Zulip chat itself (you need to join the chat). I have made a small script that fetches a fixed number of messages, not in an iterative manner. |
Hi @vchrombie , sorry for the late reply on this PR! I'm on it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @vchrombie , the PR looks promising! I left some comments and code suggestions which allow to run the backend with the info you provided here .
I understand that the support for pagination is missing, please give it a try and let me know if you find any blocker. For the next iteration, please consider to add also the docstring descriptions.
Thanks for your time!
Thanks for the suggestions and pointers @valeriocos.
I have a logic in my mind. Once I fix this issue #667 (review comment), I will head to implement it and let you know about it. 😃 |
Hi @vchrombie , I guess yesterday I make a mistake when copying the diffs, since the backend was working for me. From your current code you should apply the following diff:
This is the output I'm getting:
|
Thanks for the help @valeriocos. I got it right. 😃 Also, we have only
I was thinking of adding
Also, do you think any more fields would go good ? |
You're welcome @vchrombie ! :)
That's a good idea!
The idea of the |
Hi @valeriocos I have tested the backend and here are the results.
There were no errors while fetching the chats. I started working on adding the tests and other things need to get this PR completed. Once it is done, I will squash the commits and fix the signoff. 😬 Thanks. |
Thank you for the update @vchrombie . Please ping me when the PR is ready for review. |
Hi @valeriocos There are still some things pending. I have faced some errors. I will check on this by tomorrow. |
88b5402
to
f7d3c83
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @vchrombie , I had a look at the PR and it is in a good shape even if not finished. I left some comments, please remember to complete the docstrings and tests.
If this can be of any help, you can run the coverage locally to make sure all code is covered, :
Thanks
9714dfe
to
feedc4e
Compare
Thanks for the review @valeriocos.
I planned to do it once I fix all the tests. I will fix them for sure. 👍
Thanks, I will use it. |
f6e5997
to
264bdd3
Compare
This commit adds support to fetch messages from a Zulip server stream. Perceval can be used as $ perceval zulip '<URL>' '<STREAM>' -e '<EMAIL>' -t '<API_KEY>' The tests have been added accordingly and the usage docs are also added. Signed-off-by: Venu Vardhan Reddy Tekula <[email protected]>
Adhering to the contributing guidelines (#incubating-repositories), this work is moved to a separate repository [1] and will be maintained over there for some time. We can open this PR again and update it when we want to merge this backend into the main module. [1] Zulip Backend for Perceval: https://github.com/vchrombie/grimoirelab-perceval-zulip Best, |
Thanks! |
Fixes #630