Create a base_job class in the common library as a location to
refactor common code from client/bin/job.py and server/server_job.py
into.
Initially this just creates the base class and attempts to refactor
some of the __init__ code into it. It also tries to refactor the
code somewhat so that it's easier to test without mocking out large
amounts of external dependencies.
It also tries to standardize on what all the official publicly usable
attributes are, mostly on the basis of looking at what the existing
code actually does. In general my approach was:
* If used outside of the job class, assume it's public. If it makes
sense make it a requirement of the base class, but in some cases
(e.g. serverdir) it really only makes sense for one of the
concrete classes. In that case I define it as "OPTIONAL", which
means it can be None (but must still be defined).
* If only used within the job class, make it "private", which means
it gets a name starting with _. Code outside of the job modules
shouldn't be touching these. This includes the "constants" that
we define with all-caps in the class.
* I added unit tests which try to actually validate this, by
checking that all the correct properties are defined after
__init__. The test cases are still limited (all the test_init
cases) because of the pain of stubbing out __init__, but I will
be adding more in the future as I refactor to reduce the amount
of mocking/stubbing needed.
Overall, I don't know that this change by itself makes the code
simpler or easier to understand, but I don't think it makes things
any worse in that regard, while adding a lot more testing and
finally giving us a well-defined interface. I have more changes
planned, but as new improvements kept creeping into this change I
decided to mail it out for some review instead of letting it grow
even bigger. Some ideas of what I still intend to do in followup
patches:
* Refactor more of the directory management code into job_directory, to
get the rest of the mkdir/makedirs code into there. That way all
of the directory management is concentrated in one place.
* Refactor all the of get_state/set_state code into something that
automatically handles this with properties without a lot of
boilerplate getter and setter methods.
* Refactor the job.record related code that's duplicated into the
base class.
Risk: High
Visibility: Mostly code changes to standardize the job class interface.
A lot of the visible changes just involve privatizing attributes
that aren't used outside of job anyway, or adding in attributes
that were only defined on the client or server.
Signed-off-by: John Admanski <jadmanski@google.com>
git-svn-id: http://test.kernel.org/svn/autotest/trunk@3916 592f7852-d20e-0410-864c-8624ca9c26a4
diff --git a/server/crashcollect.py b/server/crashcollect.py
index 16e394d..916abef 100644
--- a/server/crashcollect.py
+++ b/server/crashcollect.py
@@ -123,12 +123,9 @@
@param host: The RemoteHost to collect from
"""
- if not host.job.uncollected_log_file:
- host.job.uncollected_log_file = ''
-
- if host.job and os.path.exists(host.job.uncollected_log_file):
+ if host.job:
try:
- logs = pickle.load(open(host.job.uncollected_log_file))
+ logs = host.job.get_client_logs()
for hostname, remote_path, local_path in logs:
if hostname == host.hostname:
logging.info("Retrieving logs from %s:%s into %s",