add a rack added event when a rack node is posted #439
Conversation
AlliumApotheosis
force-pushed
the
rack-event
branch
from
eb2cadf to
ed6152f
Compare
|
👍 |
|
lib/services/nodes-api-service.js
Outdated
| }; | ||
| return workflowApiService.createAndRunGraph(configuration); | ||
| } else if (node.type === Constants.NodeTypes.Rack) { | ||
| return eventsProtocol.publishNodeEvent(node, 'added') |
There was a problem hiding this comment.
maybe it's worth to extend to other types of nodes, not only for rack nodes. if you think it's fine, https://github.com/RackHD/on-http/blob/master/lib/services/nodes-api-service.js#L284 https://github.com/RackHD/on-http/blob/master/lib/services/profiles-api-service.js#L224 http://rackhd.readthedocs.io/en/latest/rackhd/event_notification.html#node-events need to be updated for publish events and update docs. I also find https://github.com/RackHD/on-http/blob/master/lib/services/profiles-api-service.js#L216 createNodeAndRunDiscovery() maybe should be located at nodes-api-service.js, not profiles-api-service.js. it could be refined. Sorry for mentioning so much, and out of your original purpose.
There was a problem hiding this comment.
I don't know if the event should be triggered for other types of nodes since there's already a 'discovered' event which should be triggered for most node types. "rack" nodes don't have a discovery process though hence this "added" event.
I think that createNodeAndRunDiscovery() is a method of profiles-api-service just for internal use?
There was a problem hiding this comment.
I think it's fine to add 'added' to other types of nodes events. it's a pre-discovered state, otherwise, it will be a little difficult for documents, or confuse users in docs.
Forget createNodeAndRunDiscovery(), let me refine it when get a chance :）
There was a problem hiding this comment.
I can do that, but the "added" event will only represent/trigger for nodes added through posts to the api.
|
👍 |
AlliumApotheosis
force-pushed
the
rack-event
branch
from
d243132 to
614388d
Compare
|
comment by test Jenkins, you can ignore this error Test Name: Http.Api.Profiles GET /profiles/switch/:vendor should throw a 400 on a request from an unknown switch vendor Test Name: Http.Api.Profiles SB 2.0 GET /profiles should send down redirect.ipxe if a node is new Test Name: Http.Api.Profiles 2.0 GET /profiles/switch/:vendor should throw a 400 on a request from an unknown switch vendor |
|
@RackHD/corecommitters Looks like this one is ready to be merged |
|
👍 |
6 participants
for https://github.com/RackHD/RackHD/wiki/proposal-rack-events-notification
@RackHD/corecommitters @pscharla