984721 - Slaveapi should abstract out Machine from Slave

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Justin Wood (:Callek)]

Attachment: [slaveapi] v1

This does the basics, as suggested by ben

There is room for improvement here, e.g.

Machine
 \-PDU
 |-OOBInterface
 |-Slave
 |-BuildbotSlave
 |-ImagingServer
 |-... etc

But this patch only deals with [Buildbot]Slave

Comment 1

[Justin Wood (:Callek)]

Comment on attachment 8392650 [details] [diff] [review]
[slaveapi] v1

Review of attachment 8392650 [details] [diff] [review]:
-----------------------------------------------------------------

::: slaveapi/slave.py
@@ -172,5 @@
>      # e.g. a foopy
> -    def load_all_info(self):
> -        self.load_inventory_info()
> -        self.load_ipmi_info()
> -        self.load_bug_info()

Just calling this out here, this change does remove "bug info" from foopies, but we're not actually using it anywhere right now. So while foopies can have their own problem tracking bugs, they rarely do and we have no need to create them here either.

Comment 2

[[e-mail] (:bhearsum)]

Comment on attachment 8392650 [details] [diff] [review]
[slaveapi] v1

Review of attachment 8392650 [details] [diff] [review]:
-----------------------------------------------------------------

Mostly fine, just a couple of things:

::: slaveapi/clients/machine.py
@@ +1,1 @@
>  import time

I'd been thinking of the clients directory as code that deals with external services, which this doesn't fit into. I'm OK with this if you feel strongly, though.

@@ -206,5 @@
>  
>      # Then wait for it come back up.
> -    return is_alive(slave, timeout=alive_timeout)
> -
> -def get_console(slave, usebuildbotslave=False):

A console seems like something that probably belongs with Machine. If you associate this with a Slave, you'll need to do the same with all the subclasses in the future.

::: slaveapi/slave.py
@@ +120,2 @@
>      # e.g. a foopy
> +    pass

I don't see any reason for this to exist. Why not just s/BuildbotSlave/Machine/ in consuming code?

Comment 3

[Justin Wood (:Callek)]

Attachment: [slaveapi] v2

Since v1:

 * Moved machine to a new folder (machines) and named the file "base"
 * Left BuildbotSlave class alone as per IRC discussion
 * Left get_console alone as per jhopkin's preference

Comment 4

[Justin Wood (:Callek)]

Attachment: [slaveapi] v2.1

forgot to adjust some import paths in v2, hence v2.1

Comment 5

[Justin Wood (:Callek)]

Comment on attachment 8394530 [details] [diff] [review]
[slaveapi] v2.1

$ git push
Counting objects: 42, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (29/29), done.
Writing objects: 100% (29/29), 4.22 KiB | 0 bytes/s, done.
Total 29 (delta 22), reused 0 (delta 0)
To ssh://git.mozilla.org/build/slaveapi.git
   1173310..ccce02a  master -> master

$ git log 1173310..ccce02a
commit ccce02ac8aa2bd7a000733a8d1211881d84fd927
Author: Justin Wood <[e-mail]>
Date:   Tue Mar 25 22:46:10 2014 -0400

    Bump slaveapi ver for Bug 921067

commit ce5eb8a162100f825b910a92c43cc7f6cb10d8e5
Author: Justin Wood <[e-mail]>
Date:   Tue Mar 25 22:44:17 2014 -0400

    Bug 921067 - Slaveapi should support panda devices for reboots. r=bhearsum

commit 8d13f0e58ce69436973a25ee2209cc09c08cf24a
Author: Justin Wood <[e-mail]>
Date:   Tue Mar 25 22:41:45 2014 -0400

    Bug 984721 - Slaveapi should abstract out Machine from Slave. r=bhearsum

commit 7b3966fd3b2904a9b51edac38239bba432ac16cc
Author: Justin Wood <[e-mail]>
Date:   Tue Mar 25 22:35:30 2014 -0400

    Bug 981039 - slaveapi should have a seperate knob for concurrency on buildapi calls alone. r=bhearsum

Comment 6

[Justin Wood (:Callek)]

had to re-bump the version for slaveapi due to a mistake here.

Needed to add slaveapi.machines to packages list in setup.py

Comment 7

[Justin Wood (:Callek)]

deployed for real as part of current Slaveapi 1.0.18.1