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

FOGL-7706: fetch plugins using notification service ip and port #1473

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from

Conversation

pintomax
Copy link
Contributor

FOGL-7706: fetch plugins using notification service ip and port

FOGL-7706: fetch plugins using notification service ip and port
Compilation fix
Unit test fixes
@pintomax
Copy link
Contributor Author

@ashwinscale @ashish-jabble please test and approve.

This fix should be added to new release as it is requested by new customers

@praveen-garg
Copy link
Member

@MarkRiddoch is this safe to consider for release at this point?

  • This is not a regression.
  • We know the workaround and design of this.
  • This will require a good amount of testing and need to be checked for all notification related endpoints.
  • A longer term fix could be considered to have consistent additional services interface via REST API.

@pintomax
Copy link
Contributor Author

pintomax commented Jan 3, 2025

@praveen-garg any update in this?

@praveen-garg
Copy link
Member

praveen-garg commented Jan 10, 2025

@pintomax I'm not sure how the things change with this. Each endpoint def in python/fledge/services/core/api/notification.py calls the URL constructed with registered notification service address.

Here get_plugin_data is added which does the same.

post_notification(request) is modified to call the new internal def; put_notification(request) still have the old mechanism?

How did you test this?

Comment on lines +61 to +63
if 'rules' not in rule_plugins:
resp['rules'] = rule_plugins
else:
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean? Are we not sure about the response object of rule_plugins from notification service?

Comment on lines +67 to +69
if 'delivery' not in delivery_plugins:
resp['delivery'] = delivery_plugins
else:
Copy link
Member

Choose a reason for hiding this comment

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

What does this mean? Are we not sure about the response object of rule_plugins from notification service?

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