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

[NFT Metadata Crawler] Fix not committing duplicate images and animations #12242

Merged
merged 8 commits into from
Feb 27, 2024

Conversation

just-in-chang
Copy link
Contributor

Description

Previously set the duplicate CDN URIs in the model but didn't write the models to DB

Also cleaned up some code

Test Plan

Works locally (different asset_uri, same raw_image_uri, copy over cdn_image_uri)
image

@just-in-chang just-in-chang requested review from bowenyang007 and a team February 26, 2024 22:15
Copy link

trunk-io bot commented Feb 26, 2024

⏱️ 13h 36m total CI duration on this PR
Job Cumulative Duration Recent Runs
rust-unit-tests 3h 32m 🟩🟩 (+4 more)
windows-build 2h 4m 🟩🟩🟩🟩🟩 (+3 more)
rust-smoke-tests 2h 1m 🟩🟥🟩
execution-performance / single-node-performance 54m 🟩🟩🟩
run-tests-main-branch 46m 🟥🟥🟥🟥 (+3 more)
rust-lints 45m 🟥🟥🟩🟩 (+3 more)
forge-e2e-test / forge 41m 🟩🟩🟩
rust-images / rust-all 39m 🟩🟩🟩
forge-compat-test / forge 38m 🟩🟥🟩
check 30m 🟩🟩🟩🟩 (+3 more)
cli-e2e-tests / run-cli-tests 20m 🟩🟩🟩
check-dynamic-deps 16m 🟩🟩🟩🟩🟩 (+3 more)
general-lints 16m 🟩🟩🟩🟩 (+3 more)
semgrep/ci 3m 🟩🟩🟩🟩🟩 (+3 more)
node-api-compatibility-tests / node-api-compatibility-tests 3m 🟩🟩🟩
file_change_determinator 2m 🟩🟩🟩🟩🟩 (+3 more)
file_change_determinator 1m 🟩🟩🟩🟩🟩 (+3 more)
execution-performance / file_change_determinator 37s 🟩🟩🟩
file_change_determinator 26s 🟩🟩🟩
permission-check 25s 🟩🟩🟩🟩🟩 (+3 more)
permission-check 24s 🟩🟩🟩🟩🟩 (+3 more)
permission-check 23s 🟩🟩🟩🟩🟩 (+3 more)
permission-check 21s 🟩🟩🟩🟩🟩 (+3 more)
permission-check 10s 🟩🟩🟩
determine-docker-build-metadata 10s 🟩🟩🟩

🚨 2 jobs on the last run were significantly faster/slower than expected

Job Duration vs 7d avg Delta
run-tests-main-branch 6m 4m +66%
windows-build 13m 20m -37%

settingsfeedbackdocs ⋅ learn more about trunk.io

}
});

if self.force || self.model.get_cdn_image_uri().is_none() && !dupe_image_found {
Copy link
Contributor

@bowenyang007 bowenyang007 Feb 27, 2024

Choose a reason for hiding this comment

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

// if self force blabla
let should_continue = if self.force {
  true
} else {
  if self.model.get_cdn_image_uri().is_some() {
    false
  } else {
    // dupe logic
  }
};

@just-in-chang just-in-chang enabled auto-merge (squash) February 27, 2024 01:09

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Copy link
Contributor

✅ Forge suite realistic_env_max_load success on dc7ce14b6d3f314884474d036c48fc1083d1408f

two traffics test: inner traffic : committed: 8141 txn/s, latency: 4824 ms, (p50: 4700 ms, p90: 5700 ms, p99: 9600 ms), latency samples: 3508960
two traffics test : committed: 100 txn/s, latency: 1812 ms, (p50: 1700 ms, p90: 2100 ms, p99: 3500 ms), latency samples: 1820
Latency breakdown for phase 0: ["QsBatchToPos: max: 0.245, avg: 0.204", "QsPosToProposal: max: 0.289, avg: 0.254", "ConsensusProposalToOrdered: max: 0.455, avg: 0.402", "ConsensusOrderedToCommit: max: 0.305, avg: 0.291", "ConsensusProposalToCommit: max: 0.702, avg: 0.693"]
Max round gap was 1 [limit 4] at version 1668155. Max no progress secs was 4.335367 [limit 15] at version 1668155.
Test Ok

Copy link
Contributor

✅ Forge suite compat success on aptos-node-v1.9.5 ==> dc7ce14b6d3f314884474d036c48fc1083d1408f

Compatibility test results for aptos-node-v1.9.5 ==> dc7ce14b6d3f314884474d036c48fc1083d1408f (PR)
1. Check liveness of validators at old version: aptos-node-v1.9.5
compatibility::simple-validator-upgrade::liveness-check : committed: 6140 txn/s, latency: 5343 ms, (p50: 4800 ms, p90: 8900 ms, p99: 17200 ms), latency samples: 221060
2. Upgrading first Validator to new version: dc7ce14b6d3f314884474d036c48fc1083d1408f
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 697 txn/s, latency: 34586 ms, (p50: 35200 ms, p90: 53700 ms, p99: 55100 ms), latency samples: 57200
3. Upgrading rest of first batch to new version: dc7ce14b6d3f314884474d036c48fc1083d1408f
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 328 txn/s, submitted: 631 txn/s, expired: 303 txn/s, latency: 36005 ms, (p50: 39900 ms, p90: 51700 ms, p99: 68000 ms), latency samples: 25304
4. upgrading second batch to new version: dc7ce14b6d3f314884474d036c48fc1083d1408f
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 2460 txn/s, latency: 12056 ms, (p50: 14300 ms, p90: 17500 ms, p99: 18000 ms), latency samples: 113160
5. check swarm health
Compatibility test for aptos-node-v1.9.5 ==> dc7ce14b6d3f314884474d036c48fc1083d1408f passed
Test Ok

@just-in-chang just-in-chang merged commit b90e264 into main Feb 27, 2024
40 of 43 checks passed
@just-in-chang just-in-chang deleted the justin/nft-dedupe-fix branch February 27, 2024 08:20
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.

3 participants