- 
                Notifications
    
You must be signed in to change notification settings  - Fork 465
 
Fix R2 tracing to use unique tags for each checksum type instead of overwriting shared tags #5252
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
Conversation
| 
           I don't think multiple checksums are allowed: https://developers.cloudflare.com/r2/api/workers/workers-api-reference/#r2putoptions 
 That said, this would be better if we ever relaxed that requirement in the future. However I think it's harder to query, so I'm a little torn.  | 
    
| 
           Alternatively, we could just set the first seen checked in a preset order. And/or post under a different tag, checksum2 or something along those lines. 
 That may be true, but I noticed that doesn't hold all the time. @jmorrell-cloudflare LMK what you prefer  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you still want to land this go for it
70ba844    to
    58a1f15      
    Compare
  
    
          
CodSpeed Performance ReportMerging #5252 will not alter performanceComparing  Summary
 Footnotes
  | 
    
| 
           @fhanau confirmed with R2 that it's standard to have multiple checksums, so it's something we should support. For ease of use, we can still designate one (picked in set order) set under some generalized checksum tag, in addition to this.  | 
    
| 
           If multiple checksums are allowed, then I'm fine merging this as-is  | 
    
3c3e8b6    to
    b1afdfc      
    Compare
  
    
No description provided.