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 clipper.exchange on Optimism #3311

Merged

Conversation

amalashkevich
Copy link
Contributor

@amalashkevich amalashkevich commented May 8, 2023

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

@Hosuke Hosuke added the dex label May 9, 2023
@Hosuke Hosuke self-assigned this May 9, 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.

@amalashkevich
Copy link
Contributor Author

Thank you, @Hosuke

@Hosuke
Copy link
Collaborator

Hosuke commented May 11, 2023

Hi @amalashkevich there may be some conflicts between branches need to be resolved.
image

Please sync/update the branch with main manually.

@amalashkevich amalashkevich requested a review from Hosuke May 11, 2023 11:00
Comment on lines 168 to 171
- check_dex_seed:
blockchain: optimism
project: clipper
version: coves_v1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here is a typo:

Suggested change
- check_dex_seed:
blockchain: optimism
project: clipper
version: coves_v1
- check_dex_seed:
blockchain: optimism
project: clipper
version: coves1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, fixed

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. for the changes

@amalashkevich
Copy link
Contributor Author

Hi @Hosuke
Could you please share when it could be merged? The deprecated queries do not show fresh data, so we need this spell.

@Hosuke
Copy link
Collaborator

Hosuke commented May 17, 2023

Hi @Hosuke Could you please share when it could be merged? The deprecated queries do not show fresh data, so we need this spell.

Hi @amalashkevich can you update the branch? We need to wait for a second reviewer for the final review.
Probably this PR will be merged in 2 days.

@Hosuke Hosuke assigned jeff-dude and unassigned Hosuke May 17, 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.

thanks all

@jeff-dude jeff-dude merged commit a15fb1f into duneanalytics:main May 17, 2023
2 checks passed
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