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.

Sign up for GitHub

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

Jump to bottom

adds celo to transfers #4060

Merged
merged 29 commits into from
Aug 24, 2023
Merged

adds celo to transfers #4060

merged 29 commits into from
Aug 24, 2023

Conversation

tomfutago
Copy link
Contributor

Thank you for contributing to Spellbook!

Please refer to the top of the readme in the root of Spellbook to learn how to contribute to Spellbook on DuneSQL.

@github-actions
Copy link

github-actions bot commented Aug 12, 2023 •
edited

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@dune-eng
Copy link

Workflow run id 5842745499 approved.

@dune-eng
Copy link

Workflow run id 5842745543 approved.

@tomfutago
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Aug 12, 2023
@github-actions
@tomfutago has signed the CLA in #4060
c999767
@dune-eng
Copy link

Workflow run id 5842851064 approved.

@dune-eng
Copy link

Workflow run id 5842851046 approved.

@dune-eng
Copy link

Workflow run id 5847716262 approved.

@dune-eng
Copy link

Workflow run id 5847716301 approved.

@dune-eng
Copy link

Workflow run id 5847827939 approved.

@dune-eng
Copy link

Workflow run id 5847828007 approved.

@dune-eng
Copy link

Workflow run id 5847941659 approved.

@dune-eng
Copy link

Workflow run id 5847941743 approved.

Hosuke
Hosuke requested changes Aug 20, 2023
View reviewed changes
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.

Hi @tomfutago ,
Do you want to add a new column block_month and we can add partition_by = ['block_month'] in the config block?

models/transfers/celo/erc20/transfers_celo_erc20.sql Outdated Show resolved Hide resolved
models/transfers/celo/transfers_celo_celo_legacy.sql Outdated Show resolved Hide resolved
@dune-eng
Copy link

Workflow run id 5918413525 approved.

@dune-eng
Copy link

Workflow run id 5918413664 approved.

@dune-eng
Copy link

Workflow run id 5918451618 approved.

@dune-eng
Copy link

Workflow run id 5918451737 approved.

@tomfutago tomfutago requested a review from Hosuke August 20, 2023 16:52
@dune-eng
Copy link

Workflow run id 5962474891 approved.

@dune-eng
Copy link

Workflow run id 5962715859 approved.

@dune-eng
Copy link

Workflow run id 5962715962 approved.

@Hosuke
Copy link
Collaborator

Hosuke commented Aug 24, 2023

hmm, passes all the tests but only with small data sample (limit 1000 on both tables), but keeps failing on larger / full datasets with our favourite error msg:

TrinoExternalError(type=EXTERNAL, name=DELTA_LAKE_BAD_WRITE, message="Unable to rewrite Parquet file"

After the test passed you can remove limit 1000.

@tomfutago tomfutago requested a review from Hosuke August 24, 2023 11:20
@dune-eng
Copy link

Workflow run id 5963932256 approved.

@dune-eng
Copy link

Workflow run id 5963932425 approved.

@dune-eng
Copy link

Workflow run id 5964107468 approved.

@dune-eng
Copy link

Workflow run id 5964107762 approved.

@dune-eng
Copy link

Workflow run id 5964149020 approved.

@dune-eng
Copy link

Workflow run id 5964148991 approved.

@dune-eng
Copy link

Workflow run id 5964291208 approved.

@dune-eng
Copy link

Workflow run id 5964291527 approved.

Hosuke
Hosuke approved these changes Aug 24, 2023
View reviewed changes
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.✅
Thank you @tomfutago

Sample transfers_celo_celo
Sample transfers_celo_erc20

@Hosuke Hosuke added ready-for-final-review and removed help wanted Extra attention is needed labels Aug 24, 2023
@Hosuke Hosuke removed their assignment Aug 24, 2023
@jeff-dude jeff-dude self-assigned this Aug 24, 2023
jeff-dude
jeff-dude approved these changes Aug 24, 2023
View reviewed changes
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.

looks good! thank you for joining us and contributing to spellbook

@jeff-dude jeff-dude merged commit c5977e1 into duneanalytics:main Aug 24, 2023
3 checks passed
@gitpoap-bot
Copy link

gitpoap-bot bot commented Aug 24, 2023

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2023 Dune Contributor:

GitPOAP: 2023 Dune Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

@github-actions github-actions bot locked and limited conversation to collaborators Aug 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Hosuke Hosuke Hosuke approved these changes

@jeff-dude jeff-dude jeff-dude approved these changes

Assignees

@jeff-dude jeff-dude

Labels
ready-for-merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@tomfutago @dune-eng @Hosuke @jeff-dude