Conversation
buildscript/sa.go
Outdated
| strings.HasSuffix(line, "but allowed_requesters is always higher precedence") || | ||
| strings.HasSuffix( | ||
| line, | ||
| "defined but not used by any variants; consider using or disabling", |
There was a problem hiding this comment.
The real change is adding these two things to the ignore list and removing the "already set in map" check, because that is no longer a warning we see.
There was a problem hiding this comment.
[narrator] But he did not, in fact, remove the "already set in map" check.
There was a problem hiding this comment.
Now I did. I also removed most of the other checks, because we're no longer getting them. It's just the unused variants one.
mmcclimon
left a comment
There was a problem hiding this comment.
I would tweak a couple things here but trust you to make these changes; I don't need to review this again unless you just really want me to, thanks!
buildscript/sa.go
Outdated
| if strings.HasPrefix(line, "WARNING: ") { | ||
| if strings.HasSuffix(line, "unmarshal errors:") || | ||
| strings.HasSuffix(line, "already set in map") || | ||
| strings.HasSuffix(line, "but allowed_requesters is always higher precedence") || |
There was a problem hiding this comment.
I think a better fix here would just be to fix the task definition (I meant to do this the other day and then got sidetracked and forgot). The fix is just in the run_manual_generated_tasks task:
diff --git a/mongodump_passthrough/tasks.yml b/mongodump_passthrough/tasks.yml
index e6ea4fde..722f258a 100644
--- a/mongodump_passthrough/tasks.yml
+++ b/mongodump_passthrough/tasks.yml
@@ -70,7 +70,7 @@ tasks:
params:
files:
- src/github.com/mongodb/mongo-tools/manual-tasks.json
- patch_only: true
+ allowed_requesters: ["patch"]
# Task dependencies have been changed slightly here, to allow this
# task to execute concurrently with "compile_coverage".(And then with that, we don't need this suffix check.)
buildscript/sa.go
Outdated
| strings.HasSuffix(line, "but allowed_requesters is always higher precedence") || | ||
| strings.HasSuffix( | ||
| line, | ||
| "defined but not used by any variants; consider using or disabling", |
There was a problem hiding this comment.
[narrator] But he did not, in fact, remove the "already set in map" check.
This also rewrites the code that ignores certain warnings to be a bit clearer.
377221a to
f911d77
Compare

This also rewrites the code that ignores certain warnings to be a bit clearer.