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 nft linked addresses #1457

Merged
merged 11 commits into from
Sep 7, 2022

Conversation

springzh
Copy link
Contributor

Brief comments on the purpose of your changes:
Added nft.linked_addresses to track all linked user addresses that buy and sell NFTs to each other, back and forth.

For Dune Engine V2
I've checked that:

  • 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

When you are ready for a review, tag duneanalytics/data-experience. We will re-open your forked pull request as an internal pull request. Then your spells will run in dbt and the logs will be avaiable in Github Actions DBT Slim CI. This job will only run the models and tests changed by your PR compared to the production project.

macros/alter_table_properties.sql Outdated Show resolved Hide resolved
@jeff-dude
Copy link
Member

@springzh it appears solana has some missing data compared to other blockchains in the buyer/seller fields. there are "dupes" based on null values:

select
  blockchain,
  linked_address_id,
  count(1)
from
  dbt_jeff_nft.linked_addresses
group by
  blockchain,
  linked_address_id
having
  count(1) > 1
order by
  3 desc

@jeff-dude
Copy link
Member

@springzh it appears solana has some missing data compared to other blockchains in the buyer/seller fields. there are "dupes" based on null values:

select
  blockchain,
  linked_address_id,
  count(1)
from
  dbt_jeff_nft.linked_addresses
group by
  blockchain,
  linked_address_id
having
  count(1) > 1
order by
  3 desc

following up on this. since there are dupes, the merge could fail in theory. are you looking into how the data contains nulls for addresses on solana / potential changes to fix it?

@dune-eng
Copy link

dune-eng commented Sep 7, 2022

Workflow run id 3004850208 approved.

@dune-eng
Copy link

dune-eng commented Sep 7, 2022

Workflow run id 3004862785 approved.

@dune-eng
Copy link

dune-eng commented Sep 7, 2022

Workflow run id 3004993798 approved.

@springzh
Copy link
Contributor Author

springzh commented Sep 7, 2022

@springzh it appears solana has some missing data compared to other blockchains in the buyer/seller fields. there are "dupes" based on null values:

select
  blockchain,
  linked_address_id,
  count(1)
from
  dbt_jeff_nft.linked_addresses
group by
  blockchain,
  linked_address_id
having
  count(1) > 1
order by
  3 desc

following up on this. since there are dupes, the merge could fail in theory. are you looking into how the data contains nulls for addresses on solana / potential changes to fix it?

Sorry for the delay.

I updated it to ignore records that contain null buyer and seller values.

Manually tested by copying the generated sql:
https://dune.com/queries/1216099
https://dune.com/queries/1245599

@jeff-dude
Copy link
Member

fyi @soispoke for deployment(s) -- i tested locally and built fresh, ran incremental, ensured no dupes, built seed and ran tests. all seem to be fine to me

@jeff-dude
Copy link
Member

the beta ci test failed because we use '+' in front of the dbt run which leads to full nft refresh and times out. i wonder if we need to alter that command on PRs @soispoke @dot2dotseurat

@jeff-dude jeff-dude merged commit 377b106 into duneanalytics:main Sep 7, 2022
@soispoke
Copy link
Contributor

soispoke commented Sep 8, 2022

@jeff-dude I think more work is needed on that front, I added the + in the dbt command otherwise it was trying to build new models without building dependencies first, but yes it turns out most additions are too heavy and time out.

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