-
Notifications
You must be signed in to change notification settings - Fork 3
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
Notification2.0 and refactoring #198
base: main
Are you sure you want to change the base?
Conversation
"http-auth": "src/nodes/http-auth/http-auth.js", | ||
"check-roles": "src/nodes/check-roles/check-roles.js" |
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.
removing the c8y-
prefix will break functionality for existing microservice users, same for the realtime nodes..
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.
Yes, I know, but how many prod users do we have? Lets have a call.
node-red-contrib-c8y-client/src/nodes/notification/notification.js
Outdated
Show resolved
Hide resolved
node-red-contrib-c8y-client/src/nodes/notification/notification.js
Outdated
Show resolved
Hide resolved
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.
In case a persistent subscription is created, you would need to also handle the deletion of that subscription whenever the node is removed.
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.
I dont know about that. Maybe the user wants to keep the messages in the queue....
This PR will introduce: