-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix exception handling #33
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,9 @@ | ||
wb-cloud-agent (1.5.9) stable; urgency=medium | ||
|
||
* Fix exception handling | ||
|
||
-- Nikolay Korotkiy <[email protected]> Thu, 15 Aug 2024 15:45:00 +0400 | ||
|
||
wb-cloud-agent (1.5.8) stable; urgency=medium | ||
|
||
* Refactor mqtt client, no functional changes | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,9 @@ | |
import json | ||
import logging | ||
import os | ||
import signal | ||
import subprocess | ||
import sys | ||
import threading | ||
import time | ||
from contextlib import ExitStack | ||
|
@@ -83,7 +85,12 @@ def do_curl(settings: AppSettings, method="get", endpoint="", params=None): | |
url, | ||
] | ||
|
||
result = subprocess.run(command, timeout=360, check=True, capture_output=True) | ||
try: | ||
result = subprocess.run(command, timeout=360, check=True, capture_output=True) | ||
except subprocess.CalledProcessError as e: | ||
raise ValueError("Error during request: " + str(e)) | ||
except subprocess.TimeoutExpired as e: | ||
raise ValueError("Timeout expired: " + str(e)) | ||
Comment on lines
+92
to
+93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. не очень понятно, зачем это заворачивать в ValueError |
||
|
||
decoded_result = result.stdout.decode("utf-8") | ||
split_result = decoded_result.split(data_delimiter) | ||
|
@@ -342,18 +349,28 @@ def main(): | |
|
||
mqtt = MQTTCloudAgent(settings, on_message) | ||
|
||
if hasattr(options, "func"): | ||
mqtt.start() | ||
return options.func(options, settings, mqtt) | ||
signal.signal(signal.SIGINT, signal.SIG_DFL) | ||
signal.signal(signal.SIGTERM, signal.SIG_DFL) | ||
|
||
if not options.daemon: | ||
mqtt.start() | ||
make_start_up_request(settings, mqtt) | ||
return show_activation_link(settings) | ||
try: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. резко всю функцию сдвинули, при этом не покрыли обработкой исключений всё то, что происходит выше (а там ведь и файлы читаются, и клиент создаётся, как будто бы тоже можно исключений наловить, ещё и таких, чтобы systemd не перезапускал это потом, а говорил, что сервис не сконфигурирован как надо) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Про это отдельно тут #38 |
||
if hasattr(options, "func"): | ||
mqtt.start() | ||
return options.func(options, settings, mqtt) | ||
|
||
mqtt.start(update_status=True) | ||
make_start_up_request(settings, mqtt) | ||
send_agent_version(settings) | ||
update_providers_list(settings, mqtt) | ||
if not options.daemon: | ||
mqtt.start() | ||
make_start_up_request(settings, mqtt) | ||
return show_activation_link(settings) | ||
|
||
run_daemon(mqtt, settings) | ||
mqtt.start(update_status=True) | ||
make_start_up_request(settings, mqtt) | ||
send_agent_version(settings) | ||
update_providers_list(settings, mqtt) | ||
|
||
run_daemon(mqtt, settings) | ||
except (ConnectionError, ConnectionRefusedError): | ||
logging.error(f"Cannot connect to broker {settings.BROKER_URL}") | ||
sys.exit(1) | ||
except Exception as e: | ||
logging.error(e) | ||
sys.exit(1) | ||
Comment on lines
+371
to
+376
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. кстати о гайдлайнах, про это мы не писали, а можно сделать сразу так, чтобы у нас был стандарт на оформление сервисов по-правильному (я так понимаю, тут ещё надо будет pylint урезонить, чтобы он не ругался на ловлю Exception) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
а зачем это заворачивать в ValueError?