Skip to content

(optionally) include location info in LoggedException log message#165

Merged
stdweird merged 8 commits intohpcugent:masterfrom
boegel:loggedexception_location
Apr 9, 2015
Merged

(optionally) include location info in LoggedException log message#165
stdweird merged 8 commits intohpcugent:masterfrom
boegel:loggedexception_location

Conversation

@boegel
Copy link
Copy Markdown
Member

@boegel boegel commented Apr 5, 2015

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 5, 2015

@stdweird: please review

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/vsc-base-pr-builder/252/
Test FAILed.

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/vsc-base-pr-builder/253/
Test FAILed.

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/vsc-base-pr-builder/254/
Test FAILed.

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/vsc-base-pr-builder/255/
Test PASSed.

Comment thread lib/vsc/utils/exceptions.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EasyBuildError -> LoggedException

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 7, 2015

@stdweird: remarks fixed or replied to, please rereview

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/vsc-base-pr-builder/258/
Test PASSed.

Comment thread lib/vsc/utils/exceptions.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see previous remarks: relpath can't have a os.path.sep, so this is always false

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copy-pasting from reply to remark, which is hidden due to the updates:

no, it's not always false, since relpath is updated via os.path.join in the body of the while loop

I'm using the os.path.sep as an 'end of name' marker to make sure I don't have a partial match, e.g.:

'easybuild/tools/filetools.py'.startswith('easybuild/') == True
'easybuild-framework/easybuild/tools/filetools.py'.startswith('easybuild/') == False

What I'm doing here is trimming something like /home/kehoste/easybuild-framework/easybuild/tools/filetools.py down to just easybuild/tools/filetools.py

I agree this looks rather strange, but I haven't found a better to do this.

path_parts is a list like ['', 'home', 'kehoste', 'easybuild-framework', 'easybuild', 'tools', 'filetools.py'], so I'm taking the end of the list until I have something that starts with easybuid/ (or vsc/)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so it's like

path_parts = frameinfo[1].split(os.path.sep)
shortest = max([path_parts.index(n) for n in self.LOC_INFO_TOP_PKG_NAMES if n in path_parts])
relpath = os.path.join(path_parts[max:])

(and deal with empty list in max etc etc)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that works too, I like it

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/vsc-base-pr-builder/260/
Test PASSed.

@boegel
Copy link
Copy Markdown
Member Author

boegel commented Apr 8, 2015

@stdweird: remark fixed, please rereview

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/vsc-base-pr-builder/261/
Test PASSed.

Comment thread lib/vsc/utils/exceptions.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replace if/else with

relpath = os.path.join(*path_parts[max(top_indices or [0]):])

(or + instead of or; it doesn't really matter)

@hpcugentbot
Copy link
Copy Markdown

Refer to this link for build results (access rights to CI server needed):
https://jenkins1.ugent.be/job/vsc-base-pr-builder/262/
Test PASSed.

stdweird added a commit that referenced this pull request Apr 9, 2015
(optionally) include location info in LoggedException log message
@stdweird stdweird merged commit e206474 into hpcugent:master Apr 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants