Skip to content
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

Add on_connect callback to mqtt listener #1033

Merged

Conversation

olekoliinyk
Copy link
Contributor

@olekoliinyk olekoliinyk commented Dec 17, 2024

Problem

test_nc_register fails in the nightly pipeline because the MQTT listener is not yet connected to the broker when the POST request is sent to register the machine.

Solution

Add on_connect callback to the MQTT listener that logs a message confirming the connection to the MQTT broker. In the test, wait for this confirmation message before sending the POST request to the core-api.

☑️ Definition of Done checklist

  • Commit history is clean (requirements)
  • README.md is updated
  • Functionality is covered by unit or integration tests

@olekoliinyk olekoliinyk requested a review from a team as a code owner December 17, 2024 14:14
@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from 1cd91a2 to 22fad2f Compare December 17, 2024 15:50
@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from 22fad2f to 3f259dc Compare December 17, 2024 17:11
@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from 7eb3cec to 2813a16 Compare December 18, 2024 08:36
@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from 2813a16 to 7ef1f81 Compare December 18, 2024 09:38
@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from 7ef1f81 to bae9dc7 Compare December 18, 2024 10:27
Jauler
Jauler previously approved these changes Dec 18, 2024
Copy link
Contributor

@Jauler Jauler left a comment

Choose a reason for hiding this comment

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

+1.0

@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from bae9dc7 to 739fce0 Compare December 18, 2024 11:57
nat-lab/tests/test_notification_center.py Outdated Show resolved Hide resolved
nat-lab/tests/test_notification_center.py Outdated Show resolved Hide resolved
nat-lab/tests/test_notification_center.py Outdated Show resolved Hide resolved
nat-lab/tests/test_notification_center.py Outdated Show resolved Hide resolved
@olekoliinyk olekoliinyk marked this pull request as draft December 18, 2024 14:51
@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from 739fce0 to 84459c6 Compare December 18, 2024 14:54
@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from 84459c6 to f64f91d Compare December 18, 2024 16:02
@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from f64f91d to ca26687 Compare December 18, 2024 16:41
@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from ca26687 to e7e2887 Compare December 18, 2024 19:42
@olekoliinyk olekoliinyk force-pushed the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch from e7e2887 to 1c4209b Compare December 18, 2024 20:38
@olekoliinyk olekoliinyk marked this pull request as ready for review December 18, 2024 20:39
Copy link
Contributor

@gytsto gytsto left a comment

Choose a reason for hiding this comment

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

looks good +1

@olekoliinyk olekoliinyk merged commit 17112c2 into main Dec 19, 2024
64 checks passed
@olekoliinyk olekoliinyk deleted the olekoliinyk/add_on_connect_callback_to_mqtt_listener branch December 19, 2024 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants