Skip to content

Value pg_probackup major_version added to the class Init #119

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 6 commits into from
Apr 10, 2024

Conversation

egarbuz
Copy link
Collaborator

@egarbuz egarbuz commented Apr 4, 2024

No description provided.

@@ -204,6 +204,8 @@ def __init__(self):
os.environ["PGAPPNAME"] = "pg_probackup"
self.delete_logs = delete_logs

self.major_version = int(self.probackup_version.split('.')[0])

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Можно более безопасные преобразования сделат, на случай если в версии что-то не то будет.Либо не скастится в int либо не будет первого элемента

if self.probackup_version.split('.')[0].isdigit:
self.major_version = int(self.probackup_version.split('.')[0])
else:
print('Pg_probackup version \"{}\" is not correct!'.format(self.probackup_version))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сообщение вида

Pg_probackup version "alpha1" is not correct

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

Expected that the major pg_probackup version should be a number, got: "alpha1" 

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправил текст выводимого сообщения об ошибке.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ошибочно утверждать, что версия некорректна, какой вид версии выбрать - это решение исключительно автора продукта. Вопрос здесь в том, что версия имеет не тот вид, который тестгрес ожидает для своей работы. Вариантом могло бы быть:

Can't process pg_probackup version "alpha1": the major version is expected to be a number

Восклицательные знаки, капслок и смайлики лучше не использовать в сообщениях для пользователя.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Обсудили, согласен с доводами.
Заменил на предложенное сообщение.

@@ -204,6 +204,12 @@ def __init__(self):
os.environ["PGAPPNAME"] = "pg_probackup"
self.delete_logs = delete_logs

if self.probackup_version.split('.')[0].isdigit:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не .isdigit, а .isdigit(). .isdigit возвращает указатель на метод, который существует у любой строки, поэтому вернёт истинное значение для любой строки:

>>> if 'abc'.isdigit:
...   print('aaa')
... 
aaa

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Поправил, спасибо.

@egarbuz egarbuz merged commit 8e06f91 into master Apr 10, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants