Opened 7 years ago

Closed 3 years ago

#2340 closed enhancement (fixed)

[MOSS Project] Please Change 'Slave' terminology

Reported by: kimmers Owned by: rutsky
Priority: patches-accepted Milestone: 0.9.4
Version: master Keywords: moss, bounty
Cc:

Description (last modified by rutsky)

I am a manager at Cray Supercomputers, where we have recently started using Buildbot. Some of our employees have found the Master/Slave? terminology offensive. Please consider changing this terminology to Master/Worker?.


Some notes on how we communicate on this bug may be salient:

  • Buildbot does have a code of conduct - please be mindful of it when posting
  • By proposing this as part of the project's MOSS application, which has been funded, we have committed to making this change. As such, arguments that Buildbot should not make this change are not particularly relevant.

--Dustin


BOUNTY: US$10,000 -- see BountyProgram


My application for this bug: https://public.etherpad-mozilla.org/p/bounty-app-2340/timeslider#1098

Remaining tasks with this issue (rutsky's TODO list):

  • [x] remove worker (deprecated) property (it was deprecated even when it was slave (deprecated)) - don't forget update relnotes about this!
  • [x] move changes that done without fallback (i.e. introduced in nine branch) from worker_transition.rst into release notes of the next beta.
  • [x] copy/rename buildbot-slave to buildbot-worker.
  • [x] remove upgrade-slave command.
  • [x] update Trac wiki (add info about buildbot-worker, fix links on docs, update "slave" in text) - done, except historical/deprecated pages
  • [x] Update master/contrib/ scripts (mostly done).

Simple cases done: https://github.com/buildbot/buildbot/pull/2557 . Hard cases probably should be removed, see issue #3658 .

  • Take a look how to push new buildbot-worker to different distributions (e.g. how to get buildbot-worker into Debian and other distros). - there is no much that can be done on my side. Packaging for distributions is responsibility of distributors maintainers. Packaging for PyPI already done.

Optional items:

  • [ ] Rework tests (make they names to be more uniform).
  • [ ] Check and rename left sb (slave builder), s (slave), sbd (slave build dir), sl, slv, sbdir, '/sl' variables.
  • [ ] rework warnings helper to use them as UnitTest?-derived class mixin.
  • [x] replace inlined class name in WorkerForBuilderBase with __class__.__name__.

--Vladimir

Change History (40)

comment:1 Changed 7 years ago by dustin

  • Milestone changed from ongoing to 1.0.+
  • Type changed from support-request to enhancement

Patches for this are welcome, but need to preserve backward and forward compatibility, without unnecessary complication in the code.

comment:2 Changed 7 years ago by Marx

The Master/Worker? definition offends me very much.

I propose as alternative PersonA/PersonBThatAgreesWithPersonAIdeasAndGladlyComplies, that should avoid anyone from being offended.

comment:3 Changed 7 years ago by dustin

Please keep the kidding out of this bug. This is a serious request, and not the first time I've heard it.

My position is that the effort required to make this change, and the user confusion that would result, *far* outweigh any increase in Buildbot's inclusiveness. If someone is willing to put in that effort to produce a patch, and it has no disadvantages for users or developers (compatibility, code complexity, user confusion), then I'm open to merging it.

And to be clear to anyone embarking on that journey: that's a high bar, and it's not clear to me it's possible to meet it.

comment:4 Changed 7 years ago by Marx

Don't you think comparing a good Worker to being owned by a Master worse than using the term Slave?

I find it very offending to our proletarian comrades.

comment:5 Changed 7 years ago by tom.prince

@Marx: This discussion isn't productive or relevant. Please take it somewhere else.

comment:6 Changed 7 years ago by Marx

<deleted>

Last edited 7 years ago by tom.prince (previous) (diff)

comment:7 Changed 7 years ago by kimmers

Master/Worker? is just a suggestion and another less offensive term may also provide an adequate solution.

@Marx There have been past legal cases where the 'Master/Slave?' term has been banned in hard drive arrangements. No matter your individual position on this subject, I think we can all agree that it would be a shame if this were to prevent someone from using this software in the future.

@dustin Thank you for being sensitive and open to this request. Note that this request would cover changes to the documentation (which is excellent by the way) as well as the code base. I do understand your concern for this to not impact development and backwards compatibility.

comment:8 Changed 7 years ago by tom.prince

One other concern, is existing third-party and mirrored documentation, that isn't under the projects control.

comment:9 Changed 7 years ago by flaviojs

master/slave... master/worker... server/worker... server/bot... hmm, buildserver/buildbot is not that bad but buildbot is already used as the master interface.

Regardless of what is chosen, this would be a radical change so it should come with a change in the major version. (1.0)

comment:10 Changed 5 years ago by cllns

I'd like to express support for this change. I understand it would require significant effort, but I think it's worth it.

The Rust programming language uses buildbot and this issue has some support there: https://github.com/rust-lang/rust-buildbot/issues/2

Here's a link to the rust-lang homepage, for those who are curious: http://www.rust-lang.org/

comment:11 Changed 4 years ago by dustin

Here's a PR for this purpose:

https://github.com/buildbot/buildbot/pull/1842

This still needs a lot of work, and I'm sure any help would be appreciated!

comment:12 Changed 4 years ago by dank

This came up at work here today. Glad to see there's some motion on it...

comment:13 Changed 4 years ago by dustin

  • Description modified (diff)
  • Keywords moss added
  • Milestone changed from 1.0.+ to 0.9.+
  • Summary changed from Please Change 'Slave' terminology to [MOSS Project] Please Change 'Slave' terminology
  • Version changed from 0.8.6p1 to master

I think that the consensus here is on renaming "Slave" to "Worker" while leaving "Master" intact -- that addresses the request without the additional complexity of changing the master's name.

I put some of my concerns in pull 1842, but it looks like that PR is stale now so I'll reiterate them here:

  • master/{slave,worker} compatibility is critical -- users with existing running buildslaves need to be able to interact with a new master. If the package name changes (buildbot-slave -> buildbot-worker), then it's fine for the reverse to not work (that is, buildbot-worker need not be compatible with buildbot versions earlier 0.9.0, as those users can still install the latest (but older) buildbot-slave package)
    • this is trickier than it sounds, since the class names are a part of the PB protocol used for master/slave communication. Another (not easy!) option is to complete the master/slave generalization and use that new protocol instead.
  • Documentation needs to be updated with some care. As a stopgap, we've endeavored to use "buildslave" instead of "slave", but that should not turn into "buildworker".
  • required changes for users should be minimized and documented clearly in the release notes and nine-upgrade.rst. For example, presumably c['slaves'] is now c['workers'] and is populated with Worker instances. If the changes are more than a simple instruction, include compatibility and a warning rather than outright failing.
  • Developer API changes need to be documented individually in the release notes. As above, prefer to warn and succeed rather than failing for un-upgraded code.
  • Modifications to the Data API do not require compatibility notes as there have been no Data API releases. However, the AngularJS frontend needs to utilize the modified API correctly.
  • Include a DB migration to rename the buildslave table and any other DB names.

As a MOSS project (and thus funded work) I will strengthen the compatibility requirement: where in any way possible, backward compatibility must be maintained, for configuration files, for existing custom buildsteps, and for network communication.

Because of the compatibility considerations, a grep of the code for /slave/ will continue to produce a number of hits. However, a new install of Buildbot should run without any user-visible evidence of the term. That includes database table names, Data API paths and fields, and any UI elements, as well as configuration files and documentation.

comment:14 Changed 4 years ago by dustin

  • Keywords bounty added

comment:15 Changed 4 years ago by dustin

  • Description modified (diff)
  • Summary changed from [MOSS Project] Please Change 'Slave' terminology to [MOSS Project] Please Change 'Slave' terminology (bounty=10,000)

comment:16 Changed 4 years ago by dustin

  • Summary changed from [MOSS Project] Please Change 'Slave' terminology (bounty=10,000) to [MOSS Project] Please Change 'Slave' terminology

comment:17 Changed 4 years ago by rutsky

Wow, 10000 for such bug is a lot, IMO. I'm in!

I think hardest part of this bug is a bunch of careful routine renaming while leaving interfaces backward compatible. Depending on how much nine interfaces are incompatible changed comparing to 0.8.*, API compatibility may be left or may be dropped. And even if API need to stay backward compatible it's not a hard problem IMO.

I'm not familiar with new master/slave protocol work-in-progress, but I debugged current master/slave protocol implementation and don't think it's too hard to backward-compatibly extend it to "master/worker" terms. So keeping current "slaves" to work with new master is definitely doable.

I would like to participate in this bounty program. Dustin, should I send e-mail to botherders with objectives and deadlines according to BountyProgram wiki page? I think I have enough experience with Buildbot to complete this project (this also be good reason to start to use nine branch for me).

comment:18 Changed 4 years ago by dustin

Yes, please do email us. Thank you!

The nascent protocol work may make protocol compatibility easier, since it isolates the class names to which Perspective Broker refers.

comment:19 Changed 4 years ago by rutsky

  • Milestone changed from 0.9.+ to 0.9.0

comment:20 Changed 4 years ago by rutsky

  • Owner set to rutsky
  • Status changed from new to assigned

comment:21 Changed 4 years ago by rutsky

  • Description modified (diff)

comment:22 Changed 4 years ago by rutsky

  • Description modified (diff)

comment:23 Changed 4 years ago by rutsky

  • Description modified (diff)

comment:24 Changed 4 years ago by rutsky

  • Description modified (diff)

comment:25 Changed 4 years ago by rutsky

  • Description modified (diff)

comment:26 Changed 3 years ago by rutsky

  • Milestone changed from 0.9.0 to 0.9.1

Most of backward incompatible changes were done for 0.9.0 release (only #3567 is left which has pending PR by Pierre)

Left changes are mostly cosmetic and may be done after 0.9.0 release, so I'm moving this bug to 0.9.1.

comment:27 Changed 3 years ago by rutsky

  • Description modified (diff)

comment:28 Changed 3 years ago by rutsky

  • Description modified (diff)

comment:29 Changed 3 years ago by tardyp

  • Milestone changed from 0.9.1 to 0.9.2

Ticket retargeted after milestone closed

comment:30 Changed 3 years ago by tardyp

  • Milestone changed from 0.9.2 to 0.9.3

Ticket retargeted after milestone closed

comment:31 Changed 3 years ago by rutsky

  • Description modified (diff)

comment:32 Changed 3 years ago by rutsky

  • Description modified (diff)

comment:33 Changed 3 years ago by rutsky

  • Description modified (diff)

comment:34 Changed 3 years ago by rutsky

  • Description modified (diff)

comment:35 Changed 3 years ago by rutsky

  • Description modified (diff)

comment:36 Changed 3 years ago by tardyp

  • Milestone changed from 0.9.3 to 0.9.4

Ticket retargeted after milestone closed

comment:37 Changed 3 years ago by rutsky

  • Description modified (diff)

comment:38 Changed 3 years ago by rutsky

  • Description modified (diff)

comment:39 Changed 3 years ago by rutsky

Looks like all required for this issue work is finally done! :)

Is there anything that I have missed or we can close this?

comment:40 Changed 3 years ago by tardyp

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.