HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python script to execute a Bash script and send a notification

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
scriptpythonandsendbashexecutenotification

Problem

I've written a Python script that given a few arguments executes a build process bash script.

```
def start(self):
"""

"""
repository = Repository.query \
.join(Client) \
.filter(Client.id == self.client_id) \
.first()

client = Client.query.filter(Client.id == self.client_id).first()

record = Build(
client_id=self.client_id,
branch=self.branch,
repository_id=repository.id,
state=self.State.PENDING.value,
started_at=datetime.datetime.now())
self._session.add(record)
self._session.commit()

wd = os.path.join(BuildServerConfig.output_dir, str(record.id))

try:
return_code = subprocess.call(args=[
'/bin/bash', BuildServerConfig.build_sh,
'--repository_url={}'.format(repository.url),
'--branch={}'.format(self.branch),
'--build_id={}'.format(record.id),
'--client_id={}'.format(self.client_id),
'--signing={}'.format(self.signing),
'--target={}'.format(self.target)
])
except Exception as exception:
logging.error(exception)
finally:
self.finalize(return_code)

final_state = None
if return_code != 0:
# Send notification email with logs
# for diagnosis attached. The recipient
# of the email is the last author that
# made the commit. It is retrieving by
# regex matching the git last log entry.
# The email is send only if the recipient's
# email address has been found.

git_dir = os.path.join(wd, 'source')
if os.path.exists(git_dir):
log = git.Git(git_dir).log('-1')
if log is not None:
pattern = r'\b[a-zA-Z0-

Solution

On the whole, it looks good.

I feel the finally clause is too long. Please break out that code into a helper function. (Or, put another way, start() is too long, and that clause appears to be low-hanging fruit.)

I wonder if it would be helpful for a with clause to accomplish these cleanup actions:

self._session.commit()
        self._session.close()


You mentioned certain frustrations with testability, for example it might be slightly challenging to arrange for return_code to be non-zero. Breaking out helper functions would make that moot, as you could just directly call such code without needing to make return_code take on a particular value.

Code Snippets

self._session.commit()
        self._session.close()

Context

StackExchange Code Review Q#154602, answer score: 3

Revisions (0)

No revisions yet.