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.

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

(WIP) keep HTML logs out of buildstatus pickles #1077

Merged
merged 8 commits into from Mar 14, 2014

Conversation

Projects
None yet
2 participants
@jpommerening
Copy link
Contributor

commented Feb 26, 2014

Trac: http://trac.buildbot.net/ticket/739

The existing HTMLLogFile class worked pretty well for the occasional stack-trace (err.html), but is not really useful for storing large HTML logs from custom buildsteps, because it keeps the whole log in the buildstatus pickle.

I tried to solve the problem by making the class inherit from LogFile, so it serializes and compresses the logfile, but added a little bit of magic to store small log files in the pickles and keep compatible with earlier versions.

In it's current implementation, HTMLLogFile will keep anything that is smaller than 10k in memory & behave (mostly) just like before. Everything beyond that threshold will be passed right through to the LogFile class.

WIP:

  • Passing very large HTML-logs into the constructor works, but is probably not a good idea, performance wise. Especially if you first have to upload the whole thing to the master and keep it in memory until you call addHtmlLog.
  • I currently can't run all tests on my box (TestRealDB segfaults) – hope this was not caused by my change.

jpommerening added some commits Feb 26, 2014

Make HTMLLogFile extend LogFile.
If logfiles were pickled with the "html" property and fake an open
logfile. Allow specifying a maximum size for logfiles to be embedded
in the pickles, otherwise just use the mechanism provided by LogFile.
fixed my indentation
That's it, I'll add it as a pre-commit hook
@djmitche

This comment has been minimized.

Copy link
Member

commented Feb 27, 2014

We'll definitely need tests for this change, including the ability to import old pickles and have them work correctly.

If TestRealDB still segfaults for the master branch, then I suspect there's a problem with your Python install, probably around the SQLite support. We can try and help in #buildbot on freenode.

jpommerening added some commits Mar 4, 2014

Merge branch 'master' into html-logs-in-pickles
* master: (47 commits)
  Fix docs/conf.py to work with sphinx-1.2.2
  pep8
  be sure run() returns a status and doesn't call finished or failed
  warn on use of old-style log methods in new-style steps
  prohibit use of step.step_status in new-style steps
  add step.*Statistic methods for new-style steps
  add docs for setStateStrings
  add step.setStateStrings
  backport ShellMixin and CommandMixin from nine
  cmd.add* return deferreds
  return deferreds from addHTMLLog and addURL
  return deferreds from addLog, addCompleteLog, and log.add*
  update docs for statuspng
  fix pngstatus test
  png status by revision hash
  fix pngstatus if the build is currently in progress
  Docs: add an 'import' in one spot to make the recipe easier to swallow
  fix docs regarding removal of LoggingBuildStep
  Fix w3c validator issues.
  Fix w3c validator issues.
  ...
Store all new logs in files.
Embedding smaller logs in the pickle was a stupid idea.
@jpommerening

This comment has been minimized.

Copy link
Contributor Author

commented Mar 4, 2014

Okay, I removed the conditional embedding of the logfiles in the pickle.
The whole HTMLLogFile class now is just a wrapper to make old pickles work like plain LogFiles.

For testing the unpickling of old html logs I checked out master, manually pickled a logfile and pasted the base64 encoded string into the test.

Let me know if you want me to rebase the PR!

PS: Yup, it was SQLite.

@djmitche

This comment has been minimized.

Copy link
Member

commented Mar 14, 2014

This looks great - thanks for sticking with it!

djmitche added a commit that referenced this pull request Mar 14, 2014

@djmitche djmitche merged commit 357ae08 into buildbot:master Mar 14, 2014

1 check passed

default The Travis CI build passed
Details

@jpommerening jpommerening deleted the jpommerening:html-logs-in-pickles branch Apr 29, 2014

thinkski pushed a commit to kuna-systems/buildbot that referenced this pull request Sep 10, 2014

if 'html' in self.__dict__:
buf = "%d:%d%s," % (len(self.html) + 1, STDERR, self.html)
self.openfile = StringIO(buf)
del self.__dict__['html']

This comment has been minimized.

Copy link
@djmitche

djmitche Apr 13, 2015

Member

Looking back at this with @mariangemarcano I notice that this doesn't write the HTML-free pickle back to disk. That might be a nice improvement for someone else (Maria?) to work on.

mariangemarcano added a commit to Unity-Technologies/katana that referenced this pull request Apr 21, 2015

Remove html log element from build pickle to improve performance when…
… loading objects

Change  HTMLLogFile this fix is based on PR buildbot#1077 upstream
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.