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

Added models for token transfers and balance for Gnosis #5926

Closed
wants to merge 107 commits into from

Conversation

hdser
Copy link
Contributor

@hdser hdser commented May 15, 2024

Thank you for contributing to Spellbook!

Thank you for taking the time to submit code in Spellbook. A few things to consider:

  • If you are a first-time contributor, please sign the CLA by copy & pasting exactly what the bot mentions in PR comment
  • Refer to docs section below to answer questions
  • Dune team will review submitted PRs as soon as possible

Best practices

To speed up your development process in PRs, keep these tips in mind:

  • Each commit to your feature branch will rerun CI tests (see example)
    • This includes all modified models on your branch
    • This includes all history of the data
  • Two tips for faster development iteration:
    • Ensure dbt is installed locally (refer to main readme) and run dbt compile
      • This will output raw SQL in target/ directory to copy/paste and run on Dune directly for initial query testing
    • Hardcode a WHERE filter for only ~7 days of history on large source tables, i.e. ethereum.transactions
      • This will speed up the CI tests and output results quicker -- whether that's an error or fully successful run
      • Once comfortable with small timeframe, remove filter and let full history run

Incremental model setup

  • Make sure your unique key columns are exactly the same in the model config block, schema yml file, and seed match columns (where applicable)
  • There cannot be nulls in the unique key columns
    • Be sure to double check key columns are correct or COALESCE() as needed on key column(s), otherwise the tests may fail on duplicates

🪄 Use the built CI tables for testing 🪄

Once CI completes, you can query the CI tables and errors in dune when it finishes running.

  • For example:
    • In the run initial models and test initial models, there will be a schema that looks like this: test_schema.git_dunesql_4da8bae_sudoswap_v2_base_pool_creations
    • This can be temporarily queried in Dune for ~24 hours

Leverage these tables to perform QA testing on Dune query editor -- or even full test dashboards!

Spellbook contribution docs

The docs directory has been implemented to answer as many questions as possible. Please take the time to reference each .md file within this directory to understand how to efficiently contribute & why the repo is designed as it is 🪄

Example questions to be answered:

Please navigate through the docs directory to find as much info as you can.

Note: happy to take PRs to improve the docs, let us know 🤝

@dune-eng
Copy link

Workflow run id 9101414919 approved.

@dune-eng
Copy link

Workflow run id 9101415158 approved.

@dune-eng
Copy link

Workflow run id 9101878016 approved.

@dune-eng
Copy link

Workflow run id 9101878188 approved.

@dune-eng
Copy link

Workflow run id 9109524394 approved.

@dune-eng
Copy link

Workflow run id 9109524722 approved.

@dune-eng
Copy link

Workflow run id 9123867224 approved.

@dune-eng
Copy link

Workflow run id 9123867410 approved.

@dune-eng
Copy link

Workflow run id 9126472351 approved.

@dune-eng
Copy link

Workflow run id 9126472167 approved.

@hdser hdser marked this pull request as draft May 17, 2024 10:03
@dune-eng
Copy link

Workflow run id 9149918728 approved.

@dune-eng
Copy link

Workflow run id 9149918772 approved.

@dune-eng
Copy link

Workflow run id 9150050147 approved.

@dune-eng
Copy link

Workflow run id 9150050172 approved.

@dune-eng
Copy link

Workflow run id 9159528070 approved.

@dune-eng
Copy link

Workflow run id 9159528314 approved.

@dune-eng
Copy link

Workflow run id 9187155608 approved.

@dune-eng
Copy link

Workflow run id 9187155834 approved.

@dune-eng
Copy link

Workflow run id 9210136693 approved.

@hdser
Copy link
Contributor Author

hdser commented Jul 15, 2024

@jeff-dude PR is ready. Suicide model is not incremental, it computes the contract balance up to the suicide call.

@dune-eng
Copy link

Workflow run id 9992794134 approved.

@dune-eng
Copy link

Workflow run id 9992794491 approved.

@dune-eng
Copy link

Workflow run id 9994154974 approved.

@dune-eng
Copy link

Workflow run id 9994155227 approved.

@dune-eng
Copy link

Workflow run id 9994600163 approved.

@dune-eng
Copy link

Workflow run id 9994600502 approved.

@dune-eng
Copy link

Workflow run id 9996472516 approved.

@dune-eng
Copy link

Workflow run id 9996472929 approved.

@dune-eng
Copy link

Workflow run id 10091128758 approved.

@dune-eng
Copy link

Workflow run id 10091129020 approved.

@dune-eng
Copy link

Workflow run id 10102258166 approved.

@dune-eng
Copy link

Workflow run id 10102258413 approved.

@jeff-dude
Copy link
Member

thank you for continued patience on this PR. i will try to revisit asap to see how we can consolidate transfers as efficiently as possible across spellbook.

@jeff-dude jeff-dude added ready-for-review this PR development is complete, please review and removed WIP work in progress in review Assignee is currently reviewing the PR labels Jul 29, 2024
@dune-eng
Copy link

Workflow run id 10160343019 approved.

@jeff-dude
Copy link
Member

sorry for the inconvenience, but we completed multi-project setup in spellbook while this PR was open. to fix merge conflicts, your code should now live in dbt_subprojects/tokens/ directory path

@hdser
Copy link
Contributor Author

hdser commented Aug 1, 2024

@jeff-dude created a new PR #6480 to accommodate the new structure.

@hdser
Copy link
Contributor Author

hdser commented Aug 1, 2024

I am closing this PR and moving to the new one

@hdser hdser closed this Aug 1, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Aug 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ready-for-review this PR development is complete, please review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants