Change in vdsm[master]: Set exc_info=True when self.message is used in log.error for...

[Haim Ateya]

Haim Ateya has uploaded a new change for review.

Change subject: Set exc_info=True when self.message is used in log.error for vds_bootstrap
......................................................................

Set exc_info=True when self.message is used in log.error for
vds_bootstrap

Change-Id: Id5fa63d9809e5f7130e304eceede6d41a27a12a6
---
M vds_bootstrap/vds_bootstrap.py
1 file changed, 7 insertions(+), 7 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/45/945/1
--
To view, visit http://gerrit.ovirt.org/945
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Id5fa63d9809e5f7130e304eceede6d41a27a12a6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Haim Ateya

[Haim Ateya]

Haim Ateya has posted comments on this change.

Change subject: Set exc_info=True when self.message is used in log.error for vds_bootstrap
......................................................................

Patch Set 1: Verified

--
To view, visit http://gerrit.ovirt.org/945
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id5fa63d9809e5f7130e304eceede6d41a27a12a6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Haim Ateya 
Gerrit-Reviewer: Haim Ateya

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Set exc_info=True when self.message is used in log.error for vds_bootstrap
......................................................................

Patch Set 1: I would prefer that you didn't submit this

(3 inline comments)

sorry, I was probably unclear when I asked you to add exc_info=True.

....................................................
File vds_bootstrap/vds_bootstrap.py
Line 539:             logging.error(self.message, exc_info=True)
this is not in an except: clause, so can cause logging of an unrelated exception.

Line 622:                         logging.error(self.message, exc_info=True)
again, should be done only in except: clauses

Line 627:                 logging.error(self.message, exc_info=True)
ditto

--
To view, visit http://gerrit.ovirt.org/945
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id5fa63d9809e5f7130e304eceede6d41a27a12a6
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Haim Ateya 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Set exc_info=True when self.message is used in log.error for vds_bootstrap
......................................................................

Patch Set 2: Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/945
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id5fa63d9809e5f7130e304eceede6d41a27a12a6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Haim Ateya 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya

Eduardo has posted comments on this change.

Change subject: Set exc_info=True when self.message is used in log.error for vds_bootstrap
......................................................................

Patch Set 2: I would prefer that you didn't submit this

(1 inline comment)

....................................................
File vds_bootstrap/vds_bootstrap.py
Line 632:             logging.error(self.message, exc_info=True)
A lot of more useful info can be returned here.
Rewrite this function, fReturn = False can't raise.

--
To view, visit http://gerrit.ovirt.org/945
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id5fa63d9809e5f7130e304eceede6d41a27a12a6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Haim Ateya 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Set exc_info=True when self.message is used in log.error for vds_bootstrap
......................................................................

Patch Set 2: (1 inline comment)

....................................................
File vds_bootstrap/vds_bootstrap.py
Line 632:             logging.error(self.message, exc_info=True)
Eduardo, this function is badly written, but it is not within Haim's mandate to rewrite it (it is within yours...)

I did not understand what you've said about fReturn

--
To view, visit http://gerrit.ovirt.org/945
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id5fa63d9809e5f7130e304eceede6d41a27a12a6
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Haim Ateya 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: Set exc_info=True when self.message is used in log.error for vds_bootstrap
......................................................................

Patch Set 3: Verified; Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/945
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id5fa63d9809e5f7130e304eceede6d41a27a12a6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Haim Ateya 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: Set exc_info=True when self.message is used in log.error for vds_bootstrap
......................................................................

Set exc_info=True when self.message is used in log.error for
vds_bootstrap

Change-Id: Id5fa63d9809e5f7130e304eceede6d41a27a12a6
---
M vds_bootstrap/vds_bootstrap.py
1 file changed, 1 insertion(+), 1 deletion(-)

Approvals:
  Dan Kenigsberg: Verified; Looks good to me, approved

--
To view, visit http://gerrit.ovirt.org/945
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Id5fa63d9809e5f7130e304eceede6d41a27a12a6
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Haim Ateya 
Gerrit-Reviewer: Dan Kenigsberg 
Gerrit-Reviewer: Eduardo 
Gerrit-Reviewer: Haim Ateya