On srpen 19th, 2014, 2:41 odp. CEST, Stephen Gallagher wrote:

src/rolekit/server/rolebase.py (Diff revision 3)
def stopServices(self):		def stop_services_async(self):
428	"""stopServices"""	438	"""stop_services_async"""
429	log.debug1("%s.stopServices()", self._log_prefix)	439	log.debug1("%s.stop_services_async()", self._log_prefix)
430	raise NotImplementedError()	440
		441	with SystemdJobHandler() as job_handler:
		442	for service in self._settings["services"]:
		443	job_path = job_handler.manager.StopUnit(service, "replace")
		444	job_handler.register_job(job_path)
		445
		446	job_results = yield job_handler.all_jobs_done_future()
		447
		448	if any([x for x in job_results.itervalues() if x not in ("skipped", "done")]):
		449	details = ", ".join(["%s: %s" % item for item in job_results.iteritems()])
		450	raise RolekitError(COMMAND_FAILED, "Stopping services failed: %s" % details)

This is almost exactly the same implementation as start_services_async. Could you merge these into a single function, please? Just pass a boolean value for starting and stopping.

There are actually three moving parts (the logging, the manager call, and the error message), adding a boolean (or more parameters) would be adding ~7 lines to save 8 lines.  Can we wait with this for restartServices?  Then we will see what precisely are the common and unique parts, and see what the cleanest way to abstract things is.  I don’t have a good intuition for this at the moment.
(This might involve s/SystemdJobHandler/SystemdServiceHandler/ and passing it the services, thus encapsulating the for loop, and a SystemdJobsResult object encapsulating the last three lines.  But I’d rather see the last user first.)

- Miloslav

On červenec 30th, 2014, 3:38 odp. CEST, Miloslav Trmac wrote:

Implement starting and stopping services, and use it in databaseserver.
Includes some work-in-progress edits to databaseserver, IMHO harmless enough but will split them if asked.
Now rereading this, should the StartUnit/StopUnit D-Bus calls be non-blocking as well?  At this late hour I’m inclined to say that they are explicitly designed to be returning quickly, so it isn’t really necessary.

starting/stopping postgres through d-feet.