-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Optimize reply by tweet Feature #4962
Comments
Any errors? Or pointers where I can have a look? |
Yes, a few places - optimization is detailed here: #3175 (comment) but as to functionality I'm not sure, I do believe we saw a Sentry error for it at one point -- here:
That's on this line: Line 386 in 5f96494
Thanks!!! |
https://sentry.io/share/issue/16c09d47269240bdaa4dd6a100e0c258/ also shows a related error! |
Sentry issue: PLOTS2-1N |
Added UserTag query optimization here: |
@jywarren all the sentry issues are related to the outbound requests i.e the request that is going out from the server to fetch the data and our system is not able to get the response. |
Can you try for the query |
Yes, it is -
https://gist.github.com/jywarren/ed5ce5c2256ac35487715223c1a46452
Hmm, what log should we look in?
…On Sat, Mar 16, 2019 at 3:07 PM Naman Gupta ***@***.***> wrote:
Can you try for the query Client.search(ENV["TWEET_SEARCH"]) in rails
console and check if that it is getting some response or not. If it not
then there will be something to be done with server configuration.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4962 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AABfJ4ojzydUSV08KUx_DV6Phy7Dwp-7ks5vXUC1gaJpZM4bmEv6>
.
|
Strange, It is working in rails console and the same line of code not working in production. |
I guess this should not happen. @jywarren can you please look into the sentry dashboard and tell the total number of time above event occurs and the last occurrence of these events? |
Well, why don't I make you a Sentry account? It looks like there are a few inter-related errors, like this one: https://sentry.io/share/issue/0e925d1fc2ab4cc7b5fca0cce5033149/ Try searching for "twitter" in the issues list there! |
Okay so I searched there and there is only one such event that had occurred. I think we can try this one more time and check if it will again give error or not. What do you think? |
I just replied, let see. Fingers Crossed! |
It neither gets added nor any sentry event comes. Strange :/ |
Since there are no errors occurs -- after seeing the log file. So there is some bug in the code which is bypassing the comment from being added. And the bug is in |
Sentry issue: PLOTS2-5B |
Aha, i found an error in Sentry that could help us? Want to take a look? |
Yeah, why not. Are you talking about https://sentry.io/organizations/publiclab/issues/976905758/?project=1410626&referrer=github_integration ? |
Till the point of retrieving tweets, code is working fine, i just had a look at http://publiclab.org/cron_log.log |
Yes, the sentry error shows line 427 of this file: Lines 420 to 427 in 01c7ace
So i wonder if it's failing to get this URL mentioned in Sentry: That's the shortcode for Twitter. Lines 397 to 399 in 01c7ace
Yeah, so for example it sees a URL like these:
Is it converting our carefully prepared URL with the NID in it into a t.co URL? Actually i haven't found many tweets from us that have that https://twitter.com/PublicLab/status/1107717566582214662 but I don't see it in the cron_log? |
Sample from the log around this tweet: https://twitter.com/PublicLab/status/1102611975949950981
|
You replied to this and it didn't get copied back, right? https://twitter.com/PublicLab/status/1105873695782957057 |
I just replied there too, let's see! Shall we log out the text it's searching for a URL in? Maybe it gets cut off? |
Lets console out more of this section, and see what it's getting! I think we may need to look specfically for |
Yes! I am going to make pull request with |
Hi @namangupta01 -- any updates on this? Thank you! |
Oh Sorry, for missing this out!
Working on it now.
Thanks
…On Fri, May 17, 2019 at 3:39 AM Jeffrey Warren ***@***.***> wrote:
Hi @namangupta01 <https://github.com/namangupta01> -- any updates on
this? Thank you!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4962?email_source=notifications&email_token=AE6AEYN5H3ZEY5JNIYWIGTLPVXLSJA5CNFSM4G4YJP5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVTGEFY#issuecomment-493249047>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AE6AEYIZTVWAWDYVNQ5A3V3PVXLSJANCNFSM4G4YJP5A>
.
|
Hi @namangupta01 can you take the next step here, we're so close! |
OK, so for testing, this tweet has the proper style URL (not converted to
|
Then following that we'd have to check how to deal with the |
I just tweeted on that tweet. Can you please share the logs so that I can see what can actually be the problem. |
OK, i see this:
Probably this is yours? |
So, if you need more info, perhaps console log out more stuff? But be sure not to share anything truly private! Thanks! |
Yup that what I was thinking to do.
Will create pull request soon.
…On Thu, 11 Jul 2019, 03:02 Jeffrey Warren, ***@***.***> wrote:
So, if you need more info, perhaps console log out more stuff? But be sure
not to share anything truly private! Thanks!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4962?email_source=notifications&email_token=AE6AEYO4OHW4VGJN375KROTP6ZIPRA5CNFSM4G4YJP5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZUZUHA#issuecomment-510237212>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AE6AEYIOA76LWTE6ELOX37TP6ZIPRANCNFSM4G4YJP5A>
.
|
Hi @namangupta01 how is this going? Thank you! |
Hi @jywarren, sorry for missing this out. I have opened the pr lets get that merge. We will be able to check what is happening in there. |
awesome, thanks!
…On Wed, Jul 24, 2019 at 2:35 PM Naman Gupta ***@***.***> wrote:
Hi @jywarren <https://github.com/jywarren>, sorry for missing this out. I
have opened the pr lets get that merge. We will be able to check what is
happening in there.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4962?email_source=notifications&email_token=AAAF6JYTQHGNQUHGYMBT4ZDQBCODJA5CNFSM4G4YJP5KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2XHEWI#issuecomment-514749017>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAAF6J6537QZ7GAGTVDSJ3TQBCODJANCNFSM4G4YJP5A>
.
|
Finally, we merged reply by tweet feature but it doesn't seem to work on Production.
The text was updated successfully, but these errors were encountered: