LINUX.ORG.RU
Форум — General  

Проба на Python

 ,


1

1

Приветствую.

Вот многие кричат что bash ненужен, бери python. Взял, и без опыта что то да состряпал.

Вот прям чувствую башизм в питоне, что сделал так, что не так, хочу критики и советов.

https://github.com/WoozyMasta/smtp-test

Помогите дельнымы советами, что бы в следующи раз делал лучше. Спасибо.

← Postfix без пользователей.
Вопрос насчет Encrypted LVM →

всё ок.

anonymous
()
  1. Если в строке есть апострофы, то чтобы не возиться с экранированием надо указывать строку в кавычках. И наоборот, если есть кавычки - то указывать в апострофах. Если и кавычки и апострофы - то в тройных кавычках.
"строка номер 'раз'"
'строка номер "два"'
"""строка номер 'три' со "сложным" форматированием"""
  1. Вместо этого
start_message = description + ' started on ' 
start_message += bind_address + ':' + str(bind_port)
start_message += ' and store logs in file \'' + log_file + '\''

можно делать строку шаблон и форматировать её подставляя параметры:

start_message_tmpl = "{description} started on {bind_address}:{bind_port} and store logs in file '{log_file}'"
start_message = start_message_tmpl.format(
  description=description,
  bind_address=bind_address,
  bind_port=bind_port,
  log_file=log_file,
)

или даже

start_message_tmpl = "{description} started on {bind_address}:{bind_port} and store logs in file '{log_file}'"
start_message = start_message_tmpl.format(**result)

  1. flake8 прогони, а то форматирование неверное в нескольких местах

  2. Я бы не стала называть переменную с входными параметрами result. Но это дело вкуса.

alpha ★★★★★
()
  1. С дефолтами немного странно.

Вместо этого:

bind_address = '0.0.0.0'
bind_port = 25
log_file = 'mail.log'
description = "Dummy smtp-test service for debuging"

...
result = parser.parse_args()
if result.bind_address:
    bind_address = result.bind_address
if result.bind_port:
    bind_port = result.bind_port
if result.log_file:
    log_file = result.log_file

можно сделать:

params = {
  bind_address: '0.0.0.0',
  bind_port: 25,
  log_file: 'mail.log',
  description = "Dummy smtp-test service for debugging",
}
...
result = parser.parse_args()
params.update(result)

Либо можно пользоваться возможностью задать default прямо в argparse:

bind_address = '0.0.0.0'
bind_port = 25
log_file = 'mail.log'
description = "Dummy smtp-test service for debuging"

parser = argparse.ArgumentParser(description=description)

parser.add_argument(
    '--host',
    dest='bind_address',
    default=bind_address,
    help='bind address or hostname',
)

parser.add_argument(
    '--port',
    dest='bind_port',
    type=int,
    default=bind_port,
    help='bind port',
)
parser.add_argument(
    '--logfile',
    dest='log_file',
    default=log_file,
    help='path to logfile',
)

parser.add_argument(
    '--loglevel',
    dest='log_level',
    default=log_level,
    help='logging level',
)

parser.add_argument('-v', '--version', action='version', 
                    version='%(prog)s 1.0', help='show version')

result = parser.parse_args()
alpha ★★★★★
()
  1. Дублирование print и log.info
print(start_message)
log.info(start_message)

Если нужно чтобы лог писался одновременно и в файл и на экран, надо просто добавить два разных Handler, один для файла (FileHandler) и один для ввода-вывода (StreamHandler), к одному и тому же логгеру.

Тогда достаточно будет один раз написать log.info(...) и получить уведомление и туда, и сюда.

alpha ★★★★★
()
  1. Слишком много всего в try-блоке

KeyboardInterrupt надо ловить там где цикл. Тут я не очень готова спорить почему, но имхо так правильнее:

def main():
    logging.basicConfig(**logopts)

    print(start_message)
    log.info(start_message)

    LoggingSMTPServer((bind_address, bind_port), None)

    try:
        asyncore.loop()
    except KeyboardInterrupt:
alpha ★★★★★
()
Ответ на: комментарий от pawnhearts

Ну это читерство. Так-то понятно, что всё уже написано до нас, но мы же скрипт ревьюим.

Кстати, @WoozyMasta, ты не против, если я буду твой пример использовать при проведении собеседований?

alpha ★★★★★
()
Ответ на: комментарий от alpha

Данные советы требуют допилинга до соответствия PEP8 :)

skiminok1986 ★★★★★
()

Нормально всё для первого опыта.

А чтобы начать писать красиво придётся читать Лутца, например. Советы в треде постольку поскольку и больше имеют отношение к вкусу, чем к реальным недостаткам.

skiminok1986 ★★★★★
()
Ответ на: комментарий от skiminok1986

Советы в треде постольку поскольку и больше имеют отношение к вкусу, чем к реальным недостаткам.

Конечно в реальной жизни никто всерьёз к простым скриптам докапываться не будет, работает и ладно.

Но тред же не о том как выполнить конкретную задачу, а о том как можно улучшить скрипт используя возможности языка.

alpha ★★★★★
()
Ответ на: комментарий от alpha

ты не против, если я буду твой пример использовать при проведении собеседований?

@alpha, конечно можно, мне не жалко) И спасибо большое за развёрнутые ответы, буду пробовать!

WoozyMasta ★
() автор топика
Ответ на: комментарий от slaykovsky

Я же не новость запостил «Сенсация, релиз первой версии отладочного SMTP»

У меня появилась разовая потребность, с задачей справился. По итогам спросил у сообщества критики.

А то что deprecated я не знал, сасибо. Может подскажешь альтернативу такую же простую в реализации?

WoozyMasta ★
() автор топика 
start_message = description + ' started on ' 
start_message += bind_address + ':' + str(bind_port)
start_message += ' and store logs in file \'' + log_file + '\''

Типичный питон, всё ок.

no-such-file ★★★★★
()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.
← Postfix без пользователей.
General
Вопрос насчет Encrypted LVM →