Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2980153002:
Milo: Fix project importer so it doesn't error out on a bad config (Closed)
|
Created:
3 years, 5 months ago by Ryan Tseng Modified:
3 years, 5 months ago Reviewers:
iannucci CC:
chromium-reviews, infra-reviews+luci-go_chromium.org, maruel+w_chromium.org, tandrii+luci-go_chromium.org Target Ref:
refs/heads/master Project:
luci-go Visibility:
Public. |
DescriptionMilo: Fix project importer so it doesn't error out on a bad config
So that a single bad config doesn't cause the importer to stop working.
Also use luci-config ids as canonical IDs in milo
Removed testing code for dupes since they're impossible now.
BUG=468053
Review-Url: https://codereview.chromium.org/2980153002
Committed: https://github.com/luci/luci-go/commit/e3f06de0b3cadca58b8fce1680fa0a2f5b602fc6
Patch Set 1 #Patch Set 2 : stuff #
Total comments: 2
Patch Set 3 : review #
Total comments: 2
Patch Set 4 : Review #
Messages
Total messages: 26 (19 generated)
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Milo: Fix project importer so it doesn't error out on a bad config So that a single bad config doesn't cause the importer to stop working BUG=468053 ========== to ========== Milo: Fix project importer so it doesn't error out on a bad config So that a single bad config doesn't cause the importer to stop working. Also use luci-config ids as canonical IDs in milo Removed testing code for dupes since they're impossible now. BUG=468053 ==========
hinoka@google.com changed reviewers: + iannucci@chromium.org
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/2980153002/diff/20001/milo/common/config.go File milo/common/config.go (right): https://codereview.chromium.org/2980153002/diff/20001/milo/common/config.go#n... milo/common/config.go:192: if err := cfgclient.Projects(c, cfgclient.AsService, cfgName, textproto.Slice(&configs), &metas); err != nil { var merr MultiError if ! nil { merr = err.(MultiError) } for i, thing := range stuff { if merr != nil && merr[i] != nil { skip } }
The CQ bit was checked by hinoka@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
https://codereview.chromium.org/2980153002/diff/20001/milo/common/config.go File milo/common/config.go (right): https://codereview.chromium.org/2980153002/diff/20001/milo/common/config.go#n... milo/common/config.go:192: if err := cfgclient.Projects(c, cfgclient.AsService, cfgName, textproto.Slice(&configs), &metas); err != nil { On 2017/07/15 00:45:53, iannucci wrote: > var merr MultiError > > if ! nil { > merr = err.(MultiError) > } > > > for i, thing := range stuff { > if merr != nil && merr[i] != nil { > skip > } > } Done.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm https://codereview.chromium.org/2980153002/diff/40001/milo/common/config.go File milo/common/config.go (right): https://codereview.chromium.org/2980153002/diff/40001/milo/common/config.go#n... milo/common/config.go:195: logging.WithError(err).Errorf(c, "Encountered error while getting project config for %s", cfgName) I would skip this since you log in the loop below.
The CQ bit was checked by hinoka@google.com to run a CQ dry run
https://codereview.chromium.org/2980153002/diff/40001/milo/common/config.go File milo/common/config.go (right): https://codereview.chromium.org/2980153002/diff/40001/milo/common/config.go#n... milo/common/config.go:195: logging.WithError(err).Errorf(c, "Encountered error while getting project config for %s", cfgName) On 2017/07/15 01:23:14, iannucci wrote: > I would skip this since you log in the loop below. Done.
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by hinoka@google.com
The CQ bit was checked by hinoka@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from iannucci@chromium.org Link to the patchset: https://codereview.chromium.org/2980153002/#ps60001 (title: "Review")
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1500081929638470,
"parent_rev": "6b67e9811353766540187a3dc80b75245c68475a", "commit_rev":
"e3f06de0b3cadca58b8fce1680fa0a2f5b602fc6"}
Message was sent while issue was closed.
Description was changed from ========== Milo: Fix project importer so it doesn't error out on a bad config So that a single bad config doesn't cause the importer to stop working. Also use luci-config ids as canonical IDs in milo Removed testing code for dupes since they're impossible now. BUG=468053 ========== to ========== Milo: Fix project importer so it doesn't error out on a bad config So that a single bad config doesn't cause the importer to stop working. Also use luci-config ids as canonical IDs in milo Removed testing code for dupes since they're impossible now. BUG=468053 Review-Url: https://codereview.chromium.org/2980153002 Committed: https://github.com/luci/luci-go/commit/e3f06de0b3cadca58b8fce1680fa0a2f5b602fc6 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://github.com/luci/luci-go/commit/e3f06de0b3cadca58b8fce1680fa0a2f5b602fc6 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
