Skip to content

Conversation

@MSilb7
Copy link
Contributor

@MSilb7 MSilb7 commented Jun 17, 2023

Brief comments on the purpose of your changes:

  • Adds Sound Edition NFT Mapping
  • Adds NFT Generated table and joins it with the optimism NFT tokens table

TODO: Add other editions factories (Mirror?)

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 5298510049 approved.

@dune-eng
Copy link

Workflow run id 5298510048 approved.

@dune-eng
Copy link

Workflow run id 5298512195 approved.

@dune-eng
Copy link

Workflow run id 5298512197 approved.

@dune-eng
Copy link

Workflow run id 5298513591 approved.

@dune-eng
Copy link

Workflow run id 5298513592 approved.

@dune-eng
Copy link

Workflow run id 5316241890 approved.

@dune-eng
Copy link

Workflow run id 5316241893 approved.

@0xRobin
Copy link
Collaborator

0xRobin commented Jun 20, 2023

Hi @MSilb7, since these models feed into tokens_nft_optimism they will be included in the code freeze after today. see #3558
We should either:

  1. move to merge this today
  2. implement it in DuneSQL after the freeze of nft.trades

@jeff-dude
Copy link
Member

i will close this until post-freeze

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants