ABANDONED: chromeos: Remove spammy "Failed to get machine id" warning.

chromeos: Remove spammy "Failed to get machine id" warning.

Remove a warning from
DeviceCloudPolicyManagerChromeOS::GetMachineID() that I see
in Chrome's log every time I boot my device after assigning
an owner:

  Failed to get machine id. This is only an error if the
  device has not yet been enrolled or claimed by a local
  user.

Callers of this method should instead log a warning
themselves if they think that there's a problem.

BUG=none

[Daniel Erat, 2016-09-16 16:45:52]

Description was changed from

==========
chromeos: Remove spamm "Failed to get machine id" warning.

Remove a warning from
DeviceCloudPolicyManagerChromeOS::GetMachineID() that I see
in Chrome's log every time I boot my device after assigning
an owner:

  Failed to get machine id. This is only an error if the
  device has not yet been enrolled or claimed by a local
  user.

Callers of this method should instead log a warning
themselves if they think that there's a problem.

BUG=none
==========

to

==========
chromeos: Remove spammy "Failed to get machine id" warning.

Remove a warning from
DeviceCloudPolicyManagerChromeOS::GetMachineID() that I see
in Chrome's log every time I boot my device after assigning
an owner:

  Failed to get machine id. This is only an error if the
  device has not yet been enrolled or claimed by a local
  user.

Callers of this method should instead log a warning
themselves if they think that there's a problem.

BUG=none
==========

[Daniel Erat, 2016-09-16 16:48:06]

derat@chromium.org changed reviewers:
+ mnissler@chromium.org, tnagel@chromium.org

[Daniel Erat, 2016-09-16 16:48:07]

this warning was expanded to its current form for http://crbug.com/545909. i'm
happy to add a more-targeted warning (i.e. one that doesn't include a "this is
only an error if ..." disclaimer) in a different place if you can point one out.

[Daniel Erat, 2016-09-16 16:48:15]

The CQ bit was checked by derat@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-16 16:48:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-16 17:29:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-16 17:29:50]

Dry run: This issue passed the CQ dry run.

[Thiemo Nagel, 2016-09-26 14:07:55]

On 2016/09/16 16:48:07, Daniel Erat wrote:
> this warning was expanded to its current form for http://crbug.com/545909. i'm
> happy to add a more-targeted warning (i.e. one that doesn't include a "this is
> only an error if ..." disclaimer) in a different place if you can point one
out.

Thanks for cleaning up logging!  I fully agree that conditionals in a warning
message are a bad thing.  I think the only critical part is
DeviceCloudPolicyInitializer::StartEnrollment(), so I'd suggest to move the
warning there (and make it an error, actually), eg.:

if (DeviceCloudPolicyManagerChromeOS::GetMachineID().empty()) { 
  LOG(ERROR) << "Failed to get machine id.";
}

[Daniel Erat, 2016-09-26 16:54:44]

On 2016/09/26 14:07:55, Thiemo Nagel (slow) wrote:
> On 2016/09/16 16:48:07, Daniel Erat wrote:
> > this warning was expanded to its current form for http://crbug.com/545909.
i'm
> > happy to add a more-targeted warning (i.e. one that doesn't include a "this
is
> > only an error if ..." disclaimer) in a different place if you can point one
> out.
> 
> Thanks for cleaning up logging!  I fully agree that conditionals in a warning
> message are a bad thing.  I think the only critical part is
> DeviceCloudPolicyInitializer::StartEnrollment(), so I'd suggest to move the
> warning there (and make it an error, actually), eg.:
> 
> if (DeviceCloudPolicyManagerChromeOS::GetMachineID().empty()) { 
>   LOG(ERROR) << "Failed to get machine id.";
> }

thanks! so you're suggesting adding this statement before the call to
manager_->core()->Disconnect() and to create the EnrollmentHandlerChromeOS
object? if there's no machine ID, should we just give up at that point rather
than starting enrollment?

[Thiemo Nagel, 2016-09-27 16:18:55]

> if there's no machine ID, should we just give up at that point rather
> than starting enrollment?

One does not simply give up inside StartEnrollment() ...  :\

Putting that up as a separate CL: https://codereview.chromium.org/2371213002/

LGTM for PS #1, but please wait for my CL to land to ensure that we're always
covered by logging.

[Thiemo Nagel, 2016-09-30 17:07:03]

Actually https://codereview.chromium.org/2371213002/ has grown to fully
supersede this CL.  Sorry...

[Daniel Erat, 2016-10-01 02:19:13]

no problem -- thanks!