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

Add Mauve - ready for review #4386

Merged
merged 11 commits into from
Sep 27, 2023
Merged

Add Mauve - ready for review #4386

merged 11 commits into from
Sep 27, 2023

Conversation

ra-phael
Copy link
Contributor

This PR adds new spells for Mauve (https://www.mauve.org/).

@github-actions
Copy link

github-actions bot commented Sep 20, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@dune-eng
Copy link

Workflow run id 6251675902 approved.

@dune-eng
Copy link

Workflow run id 6251675800 approved.

@ra-phael
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Sep 20, 2023
@dune-eng
Copy link

Workflow run id 6251849825 approved.

@dune-eng
Copy link

Workflow run id 6251849750 approved.

@dune-eng
Copy link

Workflow run id 6259377353 approved.

@dune-eng
Copy link

Workflow run id 6259377457 approved.

@dune-eng
Copy link

Workflow run id 6259730234 approved.

@dune-eng
Copy link

Workflow run id 6259730318 approved.

@dune-eng
Copy link

Workflow run id 6259988845 approved.

@dune-eng
Copy link

Workflow run id 6259988687 approved.

@dune-eng
Copy link

Workflow run id 6260093100 approved.

@dune-eng
Copy link

Workflow run id 6260093242 approved.

@dune-eng
Copy link

Workflow run id 6260762369 approved.

@dune-eng
Copy link

Workflow run id 6262059304 approved.

@dune-eng
Copy link

Workflow run id 6262441327 approved.

@dune-eng
Copy link

Workflow run id 6262440961 approved.

@ra-phael ra-phael marked this pull request as ready for review September 21, 2023 13:35
@ra-phael
Copy link
Contributor Author

I added the LsETH token in models/tokens/ethereum/tokens_ethereum_erc20.sql but mauve_ethereum_trades still have empty columns for this token when it is involved in a trade...

What am I missing? @jeff-dude

image

Sorry I'm a Dune newbie, this is like my first day in the school of Blockchain analytics and Wizardry. 😆

@dune-eng
Copy link

Workflow run id 6274269428 approved.

@dune-eng
Copy link

Workflow run id 6274269522 approved.

@dune-eng
Copy link

Workflow run id 6296976608 approved.

@dune-eng
Copy link

Workflow run id 6296976733 approved.

@ra-phael ra-phael changed the title Add Mauve Add Mauve - ready for review Sep 25, 2023
@jeff-dude
Copy link
Member

I added the LsETH token in models/tokens/ethereum/tokens_ethereum_erc20.sql but mauve_ethereum_trades still have empty columns for this token when it is involved in a trade...

What am I missing? @jeff-dude

image Sorry I'm a Dune newbie, this is like my first day in the school of Blockchain analytics and Wizardry. 😆

no need to apologize -- welcome to spellbook and thank you for contributing 🪄

i think i see the issue here and it's not anything you've done wrong. in our PR CI test actions, we only run models which are directly modified in that specific PR. if a model is referenced and not in the PR, it will read from prod instead. in this instance, you modify the tokens_ethereum_erc20 spell, but the tokens_erc20 spell is not included, which combines all blockchains into one. then in your new dex spell, you join to tokens_erc20, which will then read from prod instead (i.e. no new token).

your best bet for a quick workaround is to modify the tokens_erc20 spell to be included here. it could be a simple new line addition, remove an empty line, etc etc.

@jeff-dude jeff-dude added WIP work in progress dbt: dex covers the DEX dbt subproject labels Sep 25, 2023
@jeff-dude jeff-dude self-assigned this Sep 25, 2023
@jeff-dude jeff-dude added the in review Assignee is currently reviewing the PR label Sep 25, 2023
@ra-phael
Copy link
Contributor Author

Got it, thank you for the explanation!

@jeff-dude
Copy link
Member

Got it, thank you for the explanation!

are you good with letting this ride then, and pushing to prod? there it will follow proper dependencies & tokens would flow through.

@ra-phael
Copy link
Contributor Author

Yes, pushing to prod sounds good to me.

@dune-eng
Copy link

Workflow run id 6327781771 approved.

@dune-eng
Copy link

Workflow run id 6327781976 approved.

Copy link
Member

@jeff-dude jeff-dude left a comment

Choose a reason for hiding this comment

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

sounds good -- let's get this queued up for merge!

thanks again for the contribution

@jeff-dude jeff-dude added ready-for-merging and removed WIP work in progress in review Assignee is currently reviewing the PR labels Sep 27, 2023
@jeff-dude jeff-dude merged commit cb0a7c6 into duneanalytics:main Sep 27, 2023
@gitpoap-bot
Copy link

gitpoap-bot bot commented Sep 27, 2023

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2023 Dune Contributor:

GitPOAP: 2023 Dune Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

@github-actions github-actions bot locked and limited conversation to collaborators Sep 27, 2023
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 ready-for-merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants