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

Finalize labels/addresses/ migration #3809

Merged

Conversation

Hosuke
Copy link
Collaborator

@Hosuke Hosuke commented Jul 20, 2023

Migrated spells in this PR:

  • model.spellbook.nft_ethereum_wallet_metrics
  • model.spellbook.labels_nft_smart_trader_roi_eth
  • model.spellbook.labels_nft
  • model.spellbook.labels_airdrop*
  • model.spellbook.labels_addresses

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

i wonder if we just delete this file too, then essentially this PR does nothing but delete files

then, later, when doing the final labels roll-up spells, we could remove the refs to airdrop(s)

@Hosuke Hosuke added the ready-for-review this PR development is complete, please review label Jul 20, 2023
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @MSilb7 ,
since this static file is too large (>30 MB), do you think we can remove this file from spellbook and use csv upload?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since MSilb7 will upload airdrop_optimism_addresses_1 and airdrop_optimism_addresses_2 through csv, I am removing this file now.

@jeff-dude jeff-dude self-assigned this Aug 4, 2023
@jeff-dude jeff-dude added in review Assignee is currently reviewing the PR and removed ready-for-review this PR development is complete, please review labels Aug 4, 2023
@jeff-dude
Copy link
Member

i continued to remove files in the lineage here, both legacy and non-legacy.

this should fail on check tags step due to labels_airdrop being modified and not yet migrated. with that in mind, i think we leave this PR as ready-for-merge but also blocked until we are ready to finalize labels_addresses

@jeff-dude jeff-dude added ready-for-merging blocked labels dependencies Pull requests that update a dependency file and removed in review Assignee is currently reviewing the PR labels Aug 12, 2023
@jeff-dude
Copy link
Member

i continued to remove files in the lineage here, both legacy and non-legacy.

this should fail on check tags step due to labels_airdrop being modified and not yet migrated. with that in mind, i think we leave this PR as ready-for-merge but also blocked until we are ready to finalize labels_addresses

@Hosuke what's the status here? still blocked for now?

@Hosuke
Copy link
Collaborator Author

Hosuke commented Sep 20, 2023

i continued to remove files in the lineage here, both legacy and non-legacy.
this should fail on check tags step due to labels_airdrop being modified and not yet migrated. with that in mind, i think we leave this PR as ready-for-merge but also blocked until we are ready to finalize labels_addresses

@Hosuke what's the status here? still blocked for now?

This PR will be unblocked when labels_address is close to ready.
Or we may merge those file deletion first, and keep the roll-up spells untouched.

@Hosuke
Copy link
Collaborator Author

Hosuke commented Sep 30, 2023

I think I can finalize all the rest of labels spells in this PR, which including:

  • model.spellbook.nft_ethereum_wallet_metrics
  • model.spellbook.labels_nft_smart_trader_roi_eth
  • model.spellbook.labels_nft
  • model.spellbook.airdrop_optimism_addresses_1
  • model.spellbook.labels_airdrop_1_receivers_optimism
  • model.spellbook.labels_airdrop
  • model.spellbook.labels_addresses

@Hosuke Hosuke changed the title Labels/addresses/airdrop migration Finalize labels/addresses/ migration Sep 30, 2023
@Hosuke Hosuke added ready-for-review this PR development is complete, please review and removed ready-for-merging labels Sep 30, 2023
@Hosuke
Copy link
Collaborator Author

Hosuke commented Sep 30, 2023

Labels_addresses

@Hosuke
Copy link
Collaborator Author

Hosuke commented Oct 2, 2023

I will try to use sources mentioned here for labels_airdrops #4436 (reply in thread) :

  • dune_upload.op_airdrop1_addresses_detailed_list
  • dune_upload.op_airdrop2_addresses_detailed_list
  • dune_upload.op_airdrop3_addresses_detailed_list
  • dune_upload.op_unclaimed_airdrop_1_distribution_simple_list

Comment on lines +9 to +11
- name: op_airdrop2_addresses_detailed_list
- name: op_airdrop_3_addresses_detailed_list
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tricky for op_airdrop_3_addresses_detailed_list

Copy link
Member

Choose a reason for hiding this comment

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

fyi @MSilb7
airdrop 3 has a different naming standard, it appears, with an extra _ in there

i won't consider a blocker here, but if you ever fix, make sure to update sources file and all source usage in models downstream

@Hosuke
Copy link
Collaborator Author

Hosuke commented Oct 2, 2023

@aalan3 aalan3 requested a review from jeff-dude October 2, 2023 11:04
@jeff-dude jeff-dude added in review Assignee is currently reviewing the PR DuneSQL migration and removed ready-for-review this PR development is complete, please review blocked dependencies Pull requests that update a dependency file labels Oct 2, 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.

niiice, more good progress here. thanks as always

@jeff-dude jeff-dude removed the in review Assignee is currently reviewing the PR label Oct 2, 2023
@jeff-dude jeff-dude merged commit 2b1b736 into duneanalytics:main Oct 2, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2023
@Hosuke Hosuke deleted the labels/addresses/airdrop-migration branch October 15, 2023 10:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants