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

airdrop.claims with fix from #3418 #3434

Merged
merged 3 commits into from
Jun 5, 2023

Conversation

hildobby
Copy link
Collaborator

…418)"

This reverts commit 478ef1d.

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')

@hildobby hildobby added the WIP work in progress label May 27, 2023
@hildobby hildobby added ready-for-review this PR development is complete, please review and removed WIP work in progress labels May 30, 2023
@hildobby
Copy link
Collaborator Author

Hey @jeff-dude, I added a schema value to each airdrop model to ensure those tables have the names they should, lmk if there is anything else I need to fix here!

@hildobby hildobby requested a review from jeff-dude May 30, 2023 15:07
@jeff-dude
Copy link
Member

Hey @jeff-dude, I added a schema value to each airdrop model to ensure those tables have the names they should, lmk if there is anything else I need to fix here!

it's odd that in the original PR, the tests passed as expected, yet in prod it failed. likely since the spell already exists, then we're adding a bunch new & something is getting out of sync. i'll have to be careful and merge this one on it's own, so it's easy to monitor. i didn't see any glaring reason as to why the dependencies were broken.

@jeff-dude jeff-dude added ready-for-merging and removed ready-for-review this PR development is complete, please review labels Jun 2, 2023
@jeff-dude jeff-dude self-assigned this Jun 2, 2023
@hildobby
Copy link
Collaborator Author

hildobby commented Jun 3, 2023

Hey @jeff-dude, I added a schema value to each airdrop model to ensure those tables have the names they should, lmk if there is anything else I need to fix here!

it's odd that in the original PR, the tests passed as expected, yet in prod it failed. likely since the spell already exists, then we're adding a bunch new & something is getting out of sync. i'll have to be careful and merge this one on it's own, so it's easy to monitor. i didn't see any glaring reason as to why the dependencies were broken.

sounds good, let me know if there's anything I can help with here

@jeff-dude jeff-dude merged commit 36413a2 into duneanalytics:main Jun 5, 2023
2 checks passed
@jeff-dude
Copy link
Member

Hey @jeff-dude, I added a schema value to each airdrop model to ensure those tables have the names they should, lmk if there is anything else I need to fix here!

it's odd that in the original PR, the tests passed as expected, yet in prod it failed. likely since the spell already exists, then we're adding a bunch new & something is getting out of sync. i'll have to be careful and merge this one on it's own, so it's easy to monitor. i didn't see any glaring reason as to why the dependencies were broken.

sounds good, let me know if there's anything I can help with here

looks like it ran fine now. the prod job had an --exclude tag:ens since we have a second job that lumped together ens/labels. i removed that exclusion and the missing dependency ran as expected. we will reorg our prod jobs to try to avoid this again

olgafetisova pushed a commit to olgafetisova/spellbook that referenced this pull request Nov 14, 2023
* Revert "Revert "Create `airdrop.claims` with 38 airdrops (duneanalytics#2979)" (duneanalytics#3418)"

This reverts commit 478ef1d.

* added schema to all configs

---------

Co-authored-by: Huang Geyang <Sukebeta@outlook.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

3 participants