Status endpoints #53
Status endpoints #53
Conversation
| module.exports.applyMiddleware = app => middleware.forEach(m => m(app)); | ||
| } else if (middleware !== undefined) { | ||
| console.log( | ||
| `Expected middleware to be of type Array, but got ${middleware} instead` |
There was a problem hiding this comment.
Won't this case always be hit when there is no env.PLUGINS_MODULE set? is that desired?
There was a problem hiding this comment.
Perhaps the conditional should be else to handle all the other cases here. That would cover the case where require(env.PLUGINS_MODULE).middleware resolves to undefined. The code as it is written wouldn't export anything, nor log anything to the console.
There was a problem hiding this comment.
sorry, 🤦♂ you are right. I misread the destructuring on that assignment... disregard
There was a problem hiding this comment.
Looking at it now, I realize I may have been a little cryptic with this code. The intended behavior is that if you specify env.PLUGINS_MODULE, and it exports an object with property middleware, that property will be an array.
If it is not an array, or it is undefined, we will ignore it altogether. Plugins/middleware are completely optional and shouldn't interrupt the startup with an error.
The else if case was meant to be informational. In the event that you set the environment variable and your module exports a value, you clearly want to add middleware. So we let you know why we can't use your value.
And in the case where require(env.PLUGINS_MODULE).middleware resolves to undefined, we aren't modifying module.exports. This will result in the module exporting an empty object. To me that seems like the desirable behavior. But it's probably not clear what conventions/behaviors are expected here.
Maybe this could use more documentation and more robust logging in all cases?
This is the first change set for implementing a global notification banner. Note: This is being opened against a feature branch, as this is not the completely implementation of the change.
STATUS_URL. This will be used in an upcoming change to ping for system status and conditionally show the notification banner.Tracking Issue
flyteorg/flyte#223