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 static cex labels #1512

Merged
merged 2 commits into from
Sep 8, 2022
Merged

Add static cex labels #1512

merged 2 commits into from
Sep 8, 2022

Conversation

soispoke
Copy link
Contributor

@soispoke soispoke commented Sep 6, 2022

This PR is meant to give an example of static labels added to the spellbook repo for CEX on ethereum.

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 adding a new model, I edited the alter table macro to display new database object (table or view) in UI explorer
  • if adding a new materialized table, I edited the optimize table macro

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

@soispoke soispoke requested review from aalan3 and masquot September 6, 2022 06:26
SELECT blockchain, address, name, category, contributor, source, created_at, updated_at
FROM (VALUES
-- Binance, Source: https://etherscan.io/accounts/label/binance
(array('ethereum'),'0x3f5ce5fbfe3e9af3971dd833d26ba9b5c936f0be', 'Binance 1', 'cex', 'hildobby', 'static', timestamp('2022-08-28'), now())
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't updated_at be the same as created_at

Copy link
Member

Choose a reason for hiding this comment

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

i could see it either way, but if they're the same, then we should just have updated_at only and leave out created_at

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea here is to have created_at reflect the moment at which contributors create the query/static labels for the first time, while updated_at reflects the last date at which the table was updated/rebuilt


labels:
+schema: labels
+materialized: view
Copy link
Member

Choose a reason for hiding this comment

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

if you leave out the subdirectories (i.e. ethereum) here, will all schemas in the 'labels' directory default to 'labels'?

SELECT blockchain, address, name, category, contributor, source, created_at, updated_at
FROM (VALUES
-- Binance, Source: https://etherscan.io/accounts/label/binance
(array('ethereum'),'0x3f5ce5fbfe3e9af3971dd833d26ba9b5c936f0be', 'Binance 1', 'cex', 'hildobby', 'static', timestamp('2022-08-28'), now())
Copy link
Member

Choose a reason for hiding this comment

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

i could see it either way, but if they're the same, then we should just have updated_at only and leave out created_at

SELECT blockchain, address, name, category, contributor, source, created_at, updated_at
FROM (VALUES
-- Binance, Source: https://etherscan.io/accounts/label/binance
(array('ethereum'),'0x3f5ce5fbfe3e9af3971dd833d26ba9b5c936f0be', 'Binance 1', 'cex', 'hildobby', 'static', timestamp('2022-08-28'), now())
Copy link
Member

Choose a reason for hiding this comment

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

if this model is specific to ethereum, do we need to allow an array value for blockchain since it'll always be ethereum to match the model?

Copy link
Member

Choose a reason for hiding this comment

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

just noticed #1513 which i think clarifies my question here. while this PR may not use it, others will in the other PR, then later combined into one object

Copy link
Contributor Author

@soispoke soispoke Sep 7, 2022

Choose a reason for hiding this comment

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

Yes exactly, for some more examples you can look at #1474 as well :)

}}

-- Static Labels
SELECT * FROM {{ ref('labels_cex') }}
Copy link
Member

Choose a reason for hiding this comment

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

what are the future use cases of adding labels here, will they have the same columns and be able to just select * union?

Copy link
Member

Choose a reason for hiding this comment

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

i think #1513 may answer this as well, looks like you'll always conform to that standard

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that's also right 👍

Copy link
Contributor Author

@soispoke soispoke left a comment

Choose a reason for hiding this comment

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

Yes that's right 👍

SELECT blockchain, address, name, category, contributor, source, created_at, updated_at
FROM (VALUES
-- Binance, Source: https://etherscan.io/accounts/label/binance
(array('ethereum'),'0x3f5ce5fbfe3e9af3971dd833d26ba9b5c936f0be', 'Binance 1', 'cex', 'hildobby', 'static', timestamp('2022-08-28'), now())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea here is to have created_at reflect the moment at which contributors create the query/static labels for the first time, while updated_at reflects the last date at which the table was updated/rebuilt

SELECT blockchain, address, name, category, contributor, source, created_at, updated_at
FROM (VALUES
-- Binance, Source: https://etherscan.io/accounts/label/binance
(array('ethereum'),'0x3f5ce5fbfe3e9af3971dd833d26ba9b5c936f0be', 'Binance 1', 'cex', 'hildobby', 'static', timestamp('2022-08-28'), now())
Copy link
Contributor Author

@soispoke soispoke Sep 7, 2022

Choose a reason for hiding this comment

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

Yes exactly, for some more examples you can look at #1474 as well :)

}}

-- Static Labels
SELECT * FROM {{ ref('labels_cex') }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that's also right 👍

Copy link
Contributor Author

@soispoke soispoke left a comment

Choose a reason for hiding this comment

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

thanks @aalan3 and @jeff-dude for taking the time to review this

@soispoke soispoke requested review from jeff-dude and aalan3 September 7, 2022 04:26
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.

i think i'm good with this 👍

@soispoke
Copy link
Contributor Author

soispoke commented Sep 8, 2022

thanks @jeff-dude !

@soispoke soispoke merged commit 25b44cd into main Sep 8, 2022
@jeff-dude jeff-dude deleted the Add-static-CEX-labels- branch September 8, 2022 14:35
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.

3 participants