Closed obaroikoh closed 4 years ago
Files with Coverage Reduction | New Missed Lines | % | ||
---|---|---|---|---|
/home/travis/build/chaoss/grimoirelab-elk/grimoire_elk/utils.py | 35 | 66.16% | ||
<!-- | Total: | 35 | --> |
Totals | |
---|---|
Change from base Build 2281: | 0.08% |
Covered Lines: | 8255 |
Relevant Lines: | 10072 |
Hi @valeriocos, I have added some tests, the schema and sample raw data. I also looked at the comments in the other other PR and made some of the changes you recommended. However, I didn't handle the concern you raised wrt storing lastMessage and url meta info because I wasn't sure why we are doing it. Perheps @animeshk08 can help give more context.
Hi @obaroikoh Thank you for the updates! Great work! I had added those two fields as I found it to be unique for this backend. However, they do not provide any necessary insights hence you may skip them for the first version of this backend.Thanks!
@animeshk08 Thanks for the quick response. I will skip them as you've suggested.
Hi @obaroikoh , can I review the PR?
Sure you can. I was comparing the enriched data with slack and noticed some fields were missing, avatar and channel_topic and had to update the PR
Hi @obaroikoh , I'm starting reviewing the PR (checking the code and running the code on some rocketchat channels)
EDIT: I'm performing a long run, tomorrow morning it should be over.
Hi @obaroikoh , can I review the PR?
Sure you can. Thank you.
Hi @obaroikoh , thank you for addressing my previous comments!
I have tested the PR and works fine. I left some comments concerning the CSV, which doesn't include some attributes. After addressing this last round of comments, I think the PR can merged.
Thank you for your work and time :)
Hi @valeriocos, once again thanks for the review. I have made the changes in the comments. You can review it when you have the time.
Cheers.
Hi @obaroikoh , thank you for your time and patience. I have just reviewed the PR, it looks great.
I have submitted a PR to your fork https://github.com/obaroikoh/grimoirelab-elk/pull/1 to increase the test coverage and complete the documentation.
Hi @valeriocos, Thanks once again for the review. While doing further testing, we found that in rare cases where topic isn't set under channel_info, enrichment breaks. I have fixed that and will be updating my PR.
Ok, perfect @obaroikoh ! Can you review the PR https://github.com/obaroikoh/grimoirelab-elk/pull/1 (or include the changes in this PR)?
Once done, this PR is ready to merge.
Please feel free to review also https://github.com/chaoss/grimoirelab-sirmordred/pull/477, it adds the instructions to execute rocketchat from Mordred.
Final question :) are you interested in creating a dashboard to visualize the data extracted from Rocketchat? If this is the case, a good start would be to mimic the dashboard for Slack (https://github.com/chaoss/grimoirelab-sigils/blob/master/json/slack.json).
Ok, perfect @obaroikoh ! Can you review the PR obaroikoh#1 (or include the changes in this PR)?
I have reviewed this. I saw a line where I was adding the avatar and noticed I wasn't using the python dict get method, on line 210. Everything else looks good.
Final question :) are you interested in creating a dashboard to visualize the data extracted from Rocketchat? If this is the case, a good start would be to mimic the dashboard for Slack (https://github.com/chaoss/grimoirelab-sigils/blob/master/json/slack.json).
I have created one already :)
Please feel free to review also chaoss/grimoirelab-sirmordred#477, it adds the instructions to execute rocketchat from Mordred.
Sure, I will do that.
I have reviewed this. I saw a line where I was adding the avatar and noticed I wasn't using the python dict get method, on line 210. Everything else looks good.
Ok, perfect! You should merge the PR https://github.com/obaroikoh/grimoirelab-elk/pull/1 and then update this PR (to include the commit https://github.com/obaroikoh/grimoirelab-elk/pull/1/commits/2a1fdee2821f1f8d9814cfb3577256e25af70097). If for some reason this isn't possible, you can copy the content of the commit above to this PR and close https://github.com/obaroikoh/grimoirelab-elk/pull/1 without merging.
I have created one already :)
That's great! Would you like to contribute to Sigils by submitting a PR with the dashboard you created?
Sure, I will do that.
Thanks
Hi @valeriocos, Apologies for the delay. I see that you have merged the PR. Thanks.
Many thanks to @animeshk08.
Ok, perfect! You should merge the PR obaroikoh#1 and then update this PR (to include the commit obaroikoh@2a1fdee). If for some reason this isn't possible, you can copy the content of the commit above to this PR and close obaroikoh#1 without merging.
Was this taken care of before the merge?
That's great! Would you like to contribute to Sigils by submitting a PR with the dashboard you created?
Sure. I'd submit one this weekend.
No worries @obaroikoh ! Since we are preparing a new release I thought that it was good to include this contribution in it.
Okay great.
Great work! @obaroikoh
Hi Team,
We needed to use this PR hence this patch.
It adds grimoire_creation_date and is_edited to the enriched date and this
Cheers
Sample output