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.

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

Perpetuals Migration - #5 - Migrate Synthetix - Ready for review #4037

Merged
merged 16 commits into from
Aug 17, 2023

Conversation

henrystats
Copy link
Contributor

Thank you for contributing to Spellbook!

Please refer to the top of the readme in the root of Spellbook to learn how to contribute to Spellbook on DuneSQL.

@dune-eng
Copy link

Workflow run id 5826909669 approved.

@dune-eng
Copy link

Workflow run id 5826909736 approved.

@henrystats henrystats changed the title Perpetual Migration - #5 - Synthetix Perpetuals Migration - #5 - Migrate Synthetix Aug 10, 2023
@dune-eng
Copy link

Workflow run id 5827011773 approved.

@dune-eng
Copy link

Workflow run id 5827012026 approved.

@dune-eng
Copy link

Workflow run id 5827427719 approved.

@dune-eng
Copy link

Workflow run id 5827427822 approved.

@dune-eng
Copy link

Workflow run id 5831905954 approved.

@dune-eng
Copy link

Workflow run id 5831906113 approved.

@dune-eng
Copy link

Workflow run id 5832252737 approved.

@dune-eng
Copy link

Workflow run id 5832252745 approved.

@dune-eng
Copy link

Workflow run id 5832351559 approved.

@dune-eng
Copy link

Workflow run id 5832351410 approved.

@dune-eng
Copy link

Workflow run id 5832493754 approved.

@dune-eng
Copy link

Workflow run id 5832493960 approved.

@henrystats henrystats changed the title Perpetuals Migration - #5 - Migrate Synthetix Perpetuals Migration - #5 - Migrate Synthetix - Ready for review Aug 11, 2023
@henrystats
Copy link
Contributor Author

this is now ready for review pending when Perpetuals Migration #1 gets merged..

@MSilb7, @RplusT I'm tagging since you built the original spell... I'm migrating to dunesql and the way I've written it returns the correct details (checked the test schemas on dunesql)

The issue is there was some filtering you did whilst decoding some of the data soo I wanted to double check that I'm not missing something because all the assets & market names show up correctly to me without needing to do those filtering...

I commented the previous code for the decoded columns underneath the ones I wrote so it's easier to review, so if you could please help check that what I have is correct, so this could be reviewed, I'd really appreciate...

I'm trying to migrate the perpetuals spell (so need to migrate all the models), and also for my selfish goal of becoming an optimism NumbaNERD...

@MSilb7
Copy link
Contributor

MSilb7 commented Aug 12, 2023

this is now ready for review pending when Perpetuals Migration #1 gets merged..

@MSilb7, @RplusT I'm tagging since you built the original spell... I'm migrating to dunesql and the way I've written it returns the correct details (checked the test schemas on dunesql)

The issue is there was some filtering you did whilst decoding some of the data soo I wanted to double check that I'm not missing something because all the assets & market names show up correctly to me without needing to do those filtering...

I commented the previous code for the decoded columns underneath the ones I wrote so it's easier to review, so if you could please help check that what I have is correct, so this could be reviewed, I'd really appreciate...

I'm trying to migrate the perpetuals spell (so need to migrate all the models), and also for my selfish goal of becoming an optimism NumbaNERD...

@RplusT said he wasn't able to reply here, but I got a response form him:

"All good. If the market names are all coming out properly as you said then it should be fine. I think dunesql has a simple equivalent function to the long convoluted series of functions in sparksql that was in there originally. That was only done because of sparksql’s limitations. dunesql looks more flexible in this regard :)”

@henrystats
Copy link
Contributor Author

oke.. thank you ser @MSilb7

@dune-eng
Copy link

Workflow run id 5843983813 approved.

@dune-eng
Copy link

Workflow run id 5843985885 approved.

@dune-eng
Copy link

Workflow run id 5886110210 approved.

@dune-eng
Copy link

Workflow run id 5886110139 approved.

@dune-eng
Copy link

Workflow run id 5886114766 approved.

@dune-eng
Copy link

Workflow run id 5886114847 approved.

@dune-eng
Copy link

Workflow run id 5886178483 approved.

@dune-eng
Copy link

Workflow run id 5886178604 approved.

@dune-eng
Copy link

Workflow run id 5886281586 approved.

@dune-eng
Copy link

Workflow run id 5886281548 approved.

@jeff-dude jeff-dude self-assigned this Aug 17, 2023
@jeff-dude jeff-dude added in review Assignee is currently reviewing the PR DuneSQL migration labels Aug 17, 2023
@jeff-dude
Copy link
Member

oke.. thank you ser @MSilb7

thank you both for confirming -- can @henrystats go ahead and remove all the sql comments of old code then to keep clean? git history will always be there if needed :)

@henrystats
Copy link
Contributor Author

oke ser will do now @jeff-dude

@dune-eng
Copy link

Workflow run id 5891667281 approved.

@dune-eng
Copy link

Workflow run id 5891667472 approved.

@dune-eng
Copy link

Workflow run id 5891697317 approved.

@dune-eng
Copy link

Workflow run id 5891697425 approved.

@jeff-dude jeff-dude removed the in review Assignee is currently reviewing the PR label Aug 17, 2023
@jeff-dude jeff-dude merged commit 8c0ad9a into duneanalytics:main Aug 17, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2023
@henrystats henrystats deleted the migrate-synthesix branch August 17, 2023 15:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants