-
Notifications
You must be signed in to change notification settings - Fork 673
feat: add SanityCheckLayer to detect unexpected list responses and ex… #6747
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
base: main
Are you sure you want to change the base?
Conversation
…pose SanityCheckLayer in mod.rs (apache#6646)
39e69a6 to
8e959c2
Compare
| match self.inner.next().await? { | ||
| Some(entry) => { | ||
| let p = entry.path(); | ||
| if !p.starts_with(&self.prefix) { |
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.
Hi, our original issue is about checking for duplicate keys.
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.
Sorry, I might have misunderstood the issue. By “duplicate keys,” do you mean entries returned by the List operation that share the same path? For example, entry1.path() == entry2.path()?
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.
Yes!
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.
Yes!
Hi @Xuanwo I’ve pushed the latest updates to address the feedback.
When you have a moment, could you please take another look?
Thanks a lot!
…6646 # Conflicts: # core/core/src/layers/sanity_check.rs
…pose SanityCheckLayer in mod.rs (#6646)
Which issue does this PR close?
Closes #.6646
Rationale for this change
What changes are included in this PR?
add SanityCheckLayer to detect unexpected list responses and expose SanityCheckLayer in mod.rs
Are there any user-facing changes?
No changes to existing APIs but this add a new layer to choose