Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Parse evaluation warnings and display in separate section. #180

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Parse evaluation warnings and display in separate section. #180

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion buildbot_nix/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ def __init__(
kwargs = self.setupShellMixin(kwargs)
super().__init__(**kwargs)
self.project = project
self.observer = logobserver.BufferLogObserver()
self.observer = logobserver.BufferLogObserver(wantStderr=True)
self.addLogObserver("stdio", self.observer)
self.supported_systems = supported_systems

Expand All @@ -172,6 +172,11 @@ def run(self) -> Generator[Any, object, Any]:
# if the command passes extract the list of stages
result = cmd.results()
if result == util.SUCCESS:
log.info(self.observer.getStderr())
Copy link
Member

Choose a reason for hiding this comment

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

Could you make the eval errors more visible? As a user, I would see this when looking at the build:

image

Then click on two toggles to unveil the eval error:

image

(small nit: the whitespace in front of the first line)

Copy link
Contributor Author

@MagicRB MagicRB Jun 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Whitespace is already fixed, left over from a test and me not knowing how pre works. But i dont think we can autoexpand, ill look. But to not put it there at all, would need UI changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah i looked, we cant autoexpand without hacks. There is API to edit the UI, only to replace it :/

Copy link
Member

Choose a reason for hiding this comment

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

What about the
image
Could this be turned orange somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will check, but I dont have high hopes

Copy link
Contributor Author

@MagicRB MagicRB Jun 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Builbots UI is very configurable, if you want to add/replace views, not if you want to modify them... i'll join their IRC/something and ask, in case I missed something

self.addHTMLLog(
"Evaluation Warnings", f"<pre>{self.observer.getStderr()}</pre>"
Copy link
Member

Choose a reason for hiding this comment

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

Do you know if the string interpolation gets properly HTML-escaped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uh, definitely not, good catch! JavaScript injection attacks disaprove :)

Copy link
Member

Choose a reason for hiding this comment

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

I think the python stdlib has some helpers for that. Otherwise I am sure that buildbot has it somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, but this was more of a prototype to showcase what can be done now anyway, but i don't think it's worth merging as it is, personally.

)

# create a ShellCommand for each stage and add them to the build
jobs = []

Expand Down