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.

Sign up for GitHub

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

Jump to bottom

Continue to fix data serialization error in 3Box integration #5981

Merged
merged 3 commits into from
Feb 20, 2020
Merged

Continue to fix data serialization error in 3Box integration #5981

merged 3 commits into from
Feb 20, 2020

Conversation

think-in-universe
Copy link
Contributor

Description

Fix the testing failure in #5947, when backing up profile data to 3Box.
Resume the work in #5955

Refers/Fixes

Fix the testing failure in #5947; resume the work in #5955

Testing

Tested locally with GitCoin development environment.

This was referenced Feb 12, 2020
Merged
Merged
@think-in-universe
Copy link
Contributor Author

@octavioamu is it possible we merge this into alpha/staging first?

one of the major issues is that we don't have sufficient data locally for testing. so it's important we could validate it in alpha/staging.

not sure whether it's possible anyone could help with more testing in alpha, in order to collect the call stack (if met 500 error when export data), and query timeframe for large size profile.

@think-in-universe
Copy link
Contributor Author

@octavioamu @owocki hey guys, could please anyone help with testing in alpha for the fix, so we could move faster for the issue?

@think-in-universe think-in-universe mentioned this pull request Feb 16, 2020
Closed
5 tasks
@think-in-universe
Copy link
Contributor Author

@octavioamu @thelostone-mc heys guys, could you please help test this fix in alpha, and collect call stack or query timeframe if it fails?

@octavioamu octavioamu merged commit c3cb5cd into gitcoinco:stable Feb 20, 2020
@think-in-universe
Copy link
Contributor Author

@octavioamu it works well for my account @think-in-universe in alpha after the merge.

Could you please ask for a try from your team to verify whether the feature works or not now?

@michaelsena
Copy link

@owocki, @octavioamu, @think-in-universe - just following up here, did we ever figure out why this integration may have been corrupting profiles, which prevented people from using their 3Box at all after backing up their Gitcoin data? Wanted to see if there was anything we could do to help if not :)

@think-in-universe
Copy link
Contributor Author

think-in-universe commented Feb 29, 2020 •
edited
Loading

@michaelsena hi Michael, I didn't know that the integration corrupted profiles and prevented users from using 3Box. How did you find that? It worked well in my testing at least in the alpha version.

The major challenge in my opinion is that it's not easy to test as we need to verify the result with user profile data from production. The mock data in dev environment cannot cover quite some scenarios.

You can try as a alpha tester of GitCoin and let us know your feedback. https://gitcoin.co/settings/account?cb=alphatester

@octavioamu could anyone from the team run more testing for the feature in alpha?

@owocki
Copy link
Contributor

owocki commented Mar 9, 2020

im still getting 503s trying to upload my data in production https://bits.owocki.com/5zuyYLwW

@think-in-universe
Copy link
Contributor Author

think-in-universe commented Mar 11, 2020 •
edited
Loading

@owocki I see. let me add some performance enhancements for query or split the query into different parts.

and can we ask more guys from the team help test the feature? does it work or not for others?

@think-in-universe think-in-universe mentioned this pull request Mar 16, 2020
Merged
@think-in-universe
Copy link
Contributor Author

think-in-universe commented Mar 16, 2020 •
edited
Loading

@owocki hey Kevin, this fix #6239 splits the queries by models. Please have a try whether it works and let me know if any issues there.

@owocki
Copy link
Contributor

owocki commented Mar 16, 2020

hmmm the https://gitcoin.co/api/v0.1/profile/backup call still times out..

@owocki
Copy link
Contributor

owocki commented Mar 16, 2020

how are you testing this? might make sense to create a profile with my similar data footprint and optimize the way its storing info

@think-in-universe
Copy link
Contributor Author

think-in-universe commented Mar 17, 2020 •
edited
Loading

@owocki the call to https://gitcoin.co/api/v0.1/profile/backup should now be split into 7 separate calls (models), could you see that in the Network console?

  const startProfileDataBackup = () => {
    if (window.syncTo3Box) {
      syncTo3Box({
        onLoading: switchIcons,
        models: [ 'profile', 'grants', 'portfolio', 'bounties', 'activities', 'tips', 'feedback' ]
      });
    }
  };

Umm.... It's always a trouble for me for faking/mocking testing data. I checked with @danlipert several months ago, but it seems there's no such a script for generating testing data. Any suggestions?

I'm testing in my local dev environment (http://localhost:8000). Currently I'm creating testing data manually via GUI manipulations with local accounts.

Or I may check out the database and import some random data there to mimic your footprint. And it would also be good to create some scripts/commands to help with this.

@think-in-universe
Copy link
Contributor Author

@owocki Hi Kevin, how did you do the testing in dev environment? I think #6239 hasn't been merged, so are you testing in a staging server which could access to production data?

@owocki
Copy link
Contributor

owocki commented Mar 17, 2020 via email

@owocki
Copy link
Contributor

owocki commented Mar 17, 2020 via email

@owocki
Copy link
Contributor

owocki commented Mar 19, 2020

testing notes:

TLDR - its better than it was.

heres what the requests look like when i run this https://bits.owocki.com/mXuqDON4

here are the failures stack trces

  1. https://bits.owocki.com/4gumNpvZ
  2. https://bits.owocki.com/9ZuAKnpv

@think-in-universe
Copy link
Contributor Author

Thanks Kevin.

Umm... ETL is always time consuming as we can see here, especially when the feedback loop is long ...

The failure of Tips in https://bits.owocki.com/9ZuAKnpv and Porfolio in https://bits.owocki.com/4gumNpvZ has been fixed with the latest #6239 . The error about Porfolio should already be fixed via #5981 . It's weird the code change in #5981 is rolled back in stable branch somehow...

There're another two timeout requests (probably 'bounties' and 'profile') in your screenshot I think, it would be nice if you could help check the body of these two requests.

image

@owocki
Copy link
Contributor

owocki commented Mar 20, 2020 via email

@think-in-universe
Copy link
Contributor Author

@owocki copy the requests as cURL is fine I think. thanks.

@think-in-universe
Copy link
Contributor Author

BTW, in another project of Cerebro (gitcoinco/data-ops#42), @frankchen07 and I are reaching out to @octavioamu about whether it's possible to get access to the production data securely for someone outside the team (like me). This may also help assist the testing of this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@octavioamu octavioamu octavioamu approved these changes

@danlipert danlipert Awaiting requested review from danlipert

@thelostone-mc thelostone-mc Awaiting requested review from thelostone-mc thelostone-mc is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@think-in-universe @michaelsena @owocki @octavioamu