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 Airswap v4 ERC20 swaps #3404

Merged
merged 6 commits into from
Jun 5, 2023
Merged

Conversation

ivigamberdiev
Copy link
Contributor

Brief comments on the purpose of your changes:

For Dune Engine V2

I've checked that:

General checks:

  • I tested the query on dune.com after compiling the model with dbt compile (compiled queries are written to the target directory)
  • I used "refs" to reference other models in this repo and "sources" to reference raw or decoded tables
  • if adding a new model, I added a test
  • the filename is unique and ends with .sql
  • each sql file is a select statement and has only one view, table or function defined
  • column names are lowercase_snake_cased
  • if adding a new model, I edited the dbt project YAML file with new directory path for both models and seeds (if applicable)
  • if wanting to expose a model in the UI (Dune data explorer), I added a post-hook in the JINJA config to add metadata (blockchains, sector/project, name and contributor Dune usernames)

Pricing checks:

  • coin_id represents the ID of the coin on coinpaprika.com
  • all the coins are active on coinpaprika.com (please remove inactive ones)

Join logic:

  • if joining to base table (i.e. ethereum transactions or traces), I looked to make it an inner join if possible

Incremental logic:

  • I used is_incremental & not is_incremental jinja block filters on both base tables and decoded tables
    • where block_time >= date_trunc("day", now() - interval '1 week')
  • if joining to base table (i.e. ethereum transactions or traces), I applied join condition where block_time >= date_trunc("day", now() - interval '1 week')
  • if joining to prices view, I applied join condition where minute >= date_trunc("day", now() - interval '1 week')

@dune-eng
Copy link

Workflow run id 5072096530 approved.

@dune-eng
Copy link

Workflow run id 5072096523 approved.

@jeff-dude
Copy link
Member

jeff-dude commented May 24, 2023

hi @ivigamberdiev -- this model currently is excluded from prod due to duplicates. please see the tags=['prod_exclude'], in the config block at top of model. if you remove that tag, the model will then run in the gh action ci test in order to see the table build/test properly. you'll notice the run right now didn't actually run the model, since we exclude that tag

@jeff-dude
Copy link
Member

hi @ivigamberdiev -- this model currently is excluded from prod due to duplicates. please see the tags=['prod_exclude'], in the config block at top of model. if you remove that tag, the model will then run in the gh action ci test in order to see the table build/test properly. you'll notice the run right now didn't actually run the model, since we exclude that tag

in theory, should see duplicates occur in the gh action, unless changes in PR here fixed that

@jeff-dude jeff-dude added WIP work in progress dex labels May 24, 2023
@dune-eng
Copy link

Workflow run id 5073264140 approved.

@dune-eng
Copy link

Workflow run id 5073264136 approved.

@Hosuke Hosuke self-assigned this May 25, 2023
Copy link
Collaborator

@Hosuke Hosuke left a comment

Choose a reason for hiding this comment

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

LGTM.✅
Thank you @ivigamberdiev

Seems there is no dupes shown in dbt-test.

Sample v4 airswap_ethereum_trades

@Hosuke Hosuke added ready-for-final-review and removed WIP work in progress labels May 27, 2023
@Hosuke Hosuke assigned jeff-dude and unassigned Hosuke May 27, 2023
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.

thank you!

@jeff-dude jeff-dude merged commit 57f17b4 into duneanalytics:main Jun 5, 2023
2 checks passed
olgafetisova pushed a commit to olgafetisova/spellbook that referenced this pull request Nov 14, 2023
* Add Airswap v4 ERC20 swaps to trades

* Remove 'prod_exclude' for Airswap trades

---------

Co-authored-by: Huang Geyang <Sukebeta@outlook.com>
Co-authored-by: jeff-dude <102681548+jeff-dude@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants