3Box-Integration: Fix Profile Data Query Timeout for Kevin's Account #6239
Conversation
merge stable from gitcoinco/web
sync change from gitcoinco/web
think-in-universe
requested review from
danlipert,
octavioamu and
thelostone-mc
as code owners
|
@octavioamu @owocki hey guys, please have a look whether this fix will resolve the issues met by Kevin previously. Any more tests and feedback are welcome. |
Codecov Report
@@ Coverage Diff @@
## stable #6239 +/- ##
==========================================
- Coverage 28.26% 28.26% -0.01%
==========================================
Files 278 278
Lines 25235 25410 +175
Branches 3694 3724 +30
==========================================
+ Hits 7132 7181 +49
- Misses 17818 17948 +130
+ Partials 285 281 -4
Continue to review full report at Codecov.
|
|
Could you please help check why the unit test fails? @octavioamu @thelostone-mc https://travis-ci.org/github/gitcoinco/web/jobs/664757656
|
|
the latest upstream branch has that fixed, im pretty sure :P
…On Fri, Mar 20, 2020 at 12:32 PM think-in-universe ***@***.***> wrote:
Could you please help check why the unit test fails? @octavioamu
<https://github.com/octavioamu> @thelostone-mc
<https://github.com/thelostone-mc>
https://travis-ci.org/github/gitcoinco/web/jobs/664757656
[image: image]
<https://user-images.githubusercontent.com/46699230/77195094-0a0b7200-6b1c-11ea-9879-2181f831674c.png>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6239 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD5PCN4CKOCBWF6CJE5PJTRIOZCXANCNFSM4LMOWVVA>
.
_________________________________________
gitcoin is live and has generated over $3.5mm for Open Source Software - see
our results <https://gitcoin.co/results>
|
|
@owocki cool. thanks |
|
kewl let me know when ur ready to test :)
…On Fri, Mar 20, 2020 at 1:34 PM think-in-universe ***@***.***> wrote:
@owocki <https://github.com/owocki> cool. thanks
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6239 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD5PCLHUSWE6GDKMHZ32S3RIPAODANCNFSM4LMOWVVA>
.
_________________________________________
gitcoin is live and has generated over $3.5mm for Open Source Software - see
our results <https://gitcoin.co/results>
|
|
cool. thanks Kevin @owocki
A bit occupied so didn't catch up the two timeout issues yet, will take a look today. Also from #5981
Any suggestions for this? |
|
woooooooooooooooooooooooooooo no 500 errors! https://bits.owocki.com/7KuRZjP6 though the 3dbox calls did get 404s/401s i did sign all the messages that wree prompted to me heres the console https://bits.owocki.com/6quBoNE7 |
|
looks like a good news!! @owocki However, after checking your log, it seems the source code (or at least the static JS files) in your local development environment is not updated to the latest version in the current PR #6239 . Or at least the
@michaelsena 's team may help take a look at the 3Box connection issue I think. To me, the 3box network is not very good often, and I cannot open 3box hub sometimes. |
|
The 404s are expected behaviour @owocki - the 3box client polls the address server until the address can be correctly resolved. |
|
@owocki looks like once the static files are updated this is good to merge? @think-in-universe @msterle we don't use any kind of js package management, just the static files, so sounds like we just need to update that library and it should be working error free? |
|
@danlipert I'm not sure actually. I would prefer @owocki test again locally before we merge. And I'm not sure whether it works for others except Kevin? How many people have tested for their profile? It works for my profile and works for some mock data I build, but I can hardly know how it looks like for others, and there're some corner cases or performance issue that is hard for me to test locally without accessing to production data (or sufficient testing data). |
|
I'm actually upset and disappointed about how we're doing testing for this issue. It's really inefficient. I have pointed out this challenge of testing data from very early (around Feb 6th, when the team asked me to help fix the failure in #5947), but no one can really help take actions to bridge the gap, and shorten the feedback loop. So every time, we have to wait for the PR being merged, to validate the correctness, or waiting for Kevin's (who is busy often) response. I would suggest we learn from this issue, and figure out how to make it easy for generating testing data or how to access production safely for a contributor, or maybe the internal team who can access to the production data could do more testing and fix the failures. |
|
i think your frustrations are valid - if sanitizing the test data was an easy lift (without a lot of risk), we'd have done it already... maybe the solution is better fixtures. @danlipert i think this is good to merge from what i saw last testing session |
|
@owocki thanks Kevin. I agree better fixtures should definitely help. and what's your opinion about how to prepare data for Cerebro (gitcoinco/data-ops#42)? a bit similar to the case here. |
|
what i would usually do is just create a few test accounts with github and
import them into my local. donno if that'll work here tho..
…On Wed, Mar 25, 2020 at 11:08 AM think-in-universe ***@***.***> wrote:
@owocki <https://github.com/owocki> thanks Kevin. I agree better fixtures
should definitely help.
and what's your opinion about how to prepare data for Cerebro (
gitcoinco/data-ops#42 <gitcoinco/data-ops#42>)?
a bit similar to the case here.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6239 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD5PCLGE4PC227QSEJ4EKTRJI3BVANCNFSM4LMOWVVA>
.
_________________________________________
gitcoin is live and has generated over $3.5mm for Open Source Software - see
our results <https://gitcoin.co/results>
|
|
@owocki yeah. I did manually create accounts from GitHub for my local testing. (I didn't know there're other ways...) But I think it didn't work well for both the 3Box integration and Cerebro, since the manual creation of test accounts with GitHub cannot fully reflect the real scenarios, and it's low efficient and difficult for creating sufficient testing data manually as I tried. Basically we cannot ensure the testing coverage, or don't have enough training/testing data for ML. And all test data are cleaned up after refresh the dev environment (the DB schemas update often), maybe I need to back it up before refresh, but it increase even more cost for one-time tasks. Anyway, better fixtures should definitely be helpful. |
|
thanks @think-in-universe - will think on this a bit @gitcoinco/engineers this is 👍 to merge from my perspective (though we probably need the tests passing huh?) |
|
hey guys, could you please move faster for this issue? The feedback loop is indeed to long. |
|
@think-in-universe I'm sure you can imagine that we have a lot of different priorities right now, asking us to "move faster" is not helpful. Also, why is this against stable instead of master? |
|
@danlipert ok. considering the issue has been mainly pending on your team's actions for around 3 to 4 months (3Box team and I have been waiting for this change for 3~4 months), I want to say I hope it can move faster. and you should know the test feedback loop is indeed long as you can see from the conversation with Kevin @owocki above (and it's already 7 days after that) the reason it's against |
|
valid feedback @think-in-universe - thanks for passing it along i think that the lack of good fixtures is certainly slowing things down here, so thats to some extent an extenuating circumstance. i wish we had the cleansed test data available. but based off what i've seen it does seem that what we've done on this PR is working, so i'd love to get it merged and into an upcoming release. @gitcoinco/engineers i've added this to the 'preview pending' part of the issue board |
|
thanks for merging the feature. Is it already available in production or only available in alpha? (with https://gitcoin.co/settings/account?cb=alphatester) |
|
⚡️ A tip worth 0.65000 ETH (110.44 USD @ $169.91/ETH) has been granted to @think-in-universe for this issue from @owocki. ⚡️ Nice work @think-in-universe! Your tip has automatically been deposited in the ETH address we have on file.
|
|
|
thanks for merging the PR. talked with @RachBLondon from 3Box team, and it would be nice to see when it could be available in production. |
|
its live now! |
|
link to use it https://gitcoin.co/settings/account seems to be workin got me .. at least insofar as i dont see any obvious errors. though the ('loading' indicator)[https://bits.owocki.com/QwuKv54G] in the UI doesnt ever seem to go awy. https://bits.owocki.com/yAu2GyyW |
|
@owocki it seems being blocked at the openSpace step (it also happened to me sometimes, but not often. I thought that was due to my network only ... ) @RachBLondon Hi Rachel, do you have any advice on this? sometimes it takes a long time to openSpace or just blocked there. I reported that before, but I thought that was due to my network condition. |
|
@think-in-universe what can i do to help unblock the fixing of that?
…On Thu, Apr 9, 2020 at 8:09 AM think-in-universe ***@***.***> wrote:
@owocki <https://github.com/owocki> it seems being blocked at the
openSpace step (it also happened to me sometimes, but not often. I thought
that was due to my network only ... )
@RachBLondon <https://github.com/RachBLondon> Hi Rachel, do you have any
advice on this? sometimes it takes a long time to openSpace or just blocked
there. I reported that before, but I thought that was due to my network
condition.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#6239 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD5PCMQIUEOIX7OUVV2XSDRLXJJZANCNFSM4LMOWVVA>
.
_________________________________________
gitcoin is live and has generated over $4.0mm for Open Source Software - see
our results <https://gitcoin.co/results>
|
|
@owocki usually I need to refresh the web page and click the 3Box button again, and it will usually work. @RachBLondon may know better about this issue, which is related to 3Box APIs and network. |
|
@think-in-universe Thanks for reporting this, will investigate. |
Description
Fix the query timeout failure in #5981 reported by @owocki , when backing up profile data to 3Box.
Resume the work in #5955
Refers/Fixes
Fix the query timeout failure in #5981 reported by @owocki ; resume the work in #5955
Testing
Tested locally with GitCoin development environment.