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

dex sector subproject creation #6161

Merged
merged 69 commits into from
Jun 17, 2024
Merged

dex sector subproject creation #6161

merged 69 commits into from
Jun 17, 2024

Conversation

jeff-dude
Copy link
Member

@jeff-dude jeff-dude commented Jun 12, 2024

update:

  • dbt compile is complete on main spellbook project ✅
  • dbt compile is complete on dex project ✅
  • run subproject end-to-end (increase CI cluster if needed)
    • this may not be feasible and/or necessary. similar to nft PR, we can likely just merge with modified turned off.
  • cleanup dbt project file on main spellbook ✅
  • ...maybe more

@jeff-dude jeff-dude added WIP work in progress dune team created by dune team dbt: dex covers the DEX dbt subproject labels Jun 12, 2024
@jeff-dude jeff-dude requested a review from aalan3 June 12, 2024 23:03
@jeff-dude
Copy link
Member Author

main spellbook project ran all models successfully:
https://github.com/duneanalytics/spellbook/actions/runs/9519362533/job/26242396617

an old test failed, but we don't run in prod or ongoing basis, so not worried about it. considering main spellbook project fine in this PR.

@jeff-dude
Copy link
Member Author

jeff-dude commented Jun 14, 2024

attempting to run the full dex subproject with a dummy change to CI pipeline to remove state:modified so all runs:
1672016
https://github.com/duneanalytics/spellbook/actions/runs/9520100813/job/26244663920?pr=6161

edit:
manually canceling all other CI pipelines to not interfere, no impact should be on those in this PR other than this dummy CI change that forced them to run. they will go away when i revert CI.

@@ -1,37 +0,0 @@
-- Given a list of trades, when we look at their partial_fill, based on the type of the trade we expect partial_fill to behave differently
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we want to keep these tests?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they were valuable in development phase, but don't really add value moving forward. we don't run the tests any time and it's hyper focused on one tx. we can keep, but feels like a good opportunity to cleanse the project

This reverts commit 1672016.
@jeff-dude jeff-dude marked this pull request as ready for review June 14, 2024 19:53
@jeff-dude jeff-dude added ready-for-review this PR development is complete, please review and removed WIP work in progress labels Jun 14, 2024
@jeff-dude jeff-dude requested a review from aalan3 June 14, 2024 19:57
@jeff-dude
Copy link
Member Author

jeff-dude commented Jun 14, 2024

i'm considering this ready to go. feel free to give final review monday morning and push any changes you see fit. happy to help finalize when i'm online monday, otherwise you can do before i'm online 🤝

edit:
dbt cloud needs finalized to mimic nft with the new dex cluster

@aalan3 aalan3 merged commit 4f48010 into main Jun 17, 2024
2 of 3 checks passed
@aalan3 aalan3 deleted the dex-subproject branch June 17, 2024 09:48
@github-actions github-actions bot locked and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dbt: dex covers the DEX dbt subproject dune team created by dune team ready-for-review this PR development is complete, please review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants