Exporting ajaxWrapper and config to UI plugins

[daltonmatos]

Hello all,

this PR aims to make it possible to write more useful plugins. Plugins that are able to request info from the backend API. Let me know what you think and if I need to adjust anything.

UI plugins should be able to call the backend. Exporting ajaxWrapper and
config makes it possible to a plugin to make such a call:

const {
  ajaxWrapper,
  config
} = window.marathonPluginInterface;

this.http({
url: `${config.apiURL}v2/plugins`
}).success(...).error(...);

[orlandohohmeier]

Hey @daltonmatos, I very much like the idea of enabling plugins to fetch data from Marathon directly to support the creation of feature rich plugins.

Sharing the complete config is a good idea, and I can't think of a reason to not share the config, but we have to make sure the config is not writable nor configurable (Object.freeze) to prevent plugins from accidentally mutating the configuration. Please also check if it makes sense to omit UIVersion, though a breaking change to the interface, as the version should be part of config and I'd like to keep the interface clean.
I wouldn't share the ajax wrapper as we should try to share as little as possible to ensure the interface is easy to maintain and changes to Marathon UI don't break plugins. The ajax wrapper is something every plugin can roll them selves, but it might make sense to provide a MarathonService that provides a clean and simple interface to Marathon and could even cache responses to avoid plugins from requesting the same information over and over. What do you think?

[daltonmatos]

Hello @orlandohohmeier. Totally agree! Didn't know about the Object.freeze(), I will adjust this.

About the UIVersion, makes sense to remove and leave only the config.version, after all, they are the same value. What is the best way to document this breaking change?

Also agree on the MarathonService object. What do you think should be the exposed interface?

MararthonService.request({}, success_callback, error_callback)

Or should it be the builder style (as it is on ajaxWrapper)?

MararthonService.request({})
.success(function(){})
.error(function(){})

Thinking about the code, it would be a direct call to ajaxWrapper() re-passing all 3 parameters: Object that configures the request, the success callback and the error callback.

Does that make sense?

Let me know what you think and I will write the code. In the mean time, I will adjust this PR with the changes we already agreed on.

Thanks,

[daltonmatos]

Hello @orlandohohmeier, any thoughts on the MarathonService interface? Would be great to agree on one interface so we avoid any reworks in the future.

Thanks,

[daltonmatos]

Hello @orlandohohmeier,

how about removing ajaxWrapper from this PR? We may let this implementation to a future PR, what do you think?

Yet, how do we properly document this breaking change about removing UIVersion?

Thanks,

[orlandohohmeier]

Hi @daltonmatos,

please excuse the late response, I haven't had the time to think about your proposal any sooner.

I really like where the MararthonService discussion is going! Such a service will provide a clean interface to request resources from Marathon and mitigates the need to share the config and any other data with any of the registered plugins. Not sharing the config and is preferable as we otherwise risk having a bunch of plugins requesting the same data over and over again without an instance that we can use to cache resources.

As propose I'd probably expose an interface very similar to the ajaxWrapper. The service should be hardwired to Marathon so that a user doesn't need to know the exact URI.

Example

// fetching groups with defaults
MararthonService.request({
    resouce: '/groups'
})
.success(function(response) {})
.error(function(response) {})

// fetching groups data that is no than 500ms  
MararthonService.request({
    resouce: '/groups',
    age: 500
})
.success(function(response) {})
.error(function(response) {})

// deploying a new app
MararthonService.request({
    resouce: '/apps'
    data: {new_app},
    method: 'POST',
    age: 500
})
.success(function(response) {})
.error(function(response) {})

Possible Options

{string} resource: resource address e.g., /groups
{object} data: object to send along with your request
{string} method (default: "GET"): request method
{number} age (default: 500): resource age – this property enables dynamic per resource caching

Next Steps

• Agree on MarathonService interface
• Implement basic service (basically an ajaxWrapper wrapper 😃 )
• Implement caching mechanism

What do you think?

/cc @mesosphere/frontend

[daltonmatos]

That's indeed a great idea @orlandohohmeier.

I have a doubt about the age parameter when used with "write-requests" (PUT, POST, DELETE). What would this parametr do for a POST request? Only create a new app if the last created app is older than 500ms?

I was thinking about this MarathonService more as a tool to make raw http requests to the marathon backend, but it can serve much more than that. What do you think about these ideas:

• Use MarathonService as a more complex/richier object. Instead of calling MararthonService.request({}) to create a new app we could call

MarathonService.createApp({
    data: {app_data},
    otherOption: ...,
    ...
})
.success(function(response) {})
.error( function(response) {})

This gives room to changes to backend API without breaking plugins. Also a plugin developer does not have to worry about the details of the underlying API, it would be able to always call createApp(). Also we would have .restartApp(), destroyApp(), suspendApp(), etc.

• Use this MarathonService to efectively be the API Exported to Plugins. Instead of just having an http service wrapper we could have a more generic namespaced object. Currently, the UI exports PluginHelper and PluginActions. These two are used to create an alert dialog. How about something like this:

MarathonService.dialogs.alert([{
    title: "Dialog Title",
    message: "Dialog Message",
   otherOptions: ...
}]);

Of course, MarathonService would probably be renamed to a more appropriate name.
This object could also be versioned (independently of the UI) so plugins can check its own compatibility against the "PluginAPI" that's being exported to them.

And this goes on and on, this object could have methods to let plugins subscribe to internal UI events, dispatch UI events, mount new components, modify requests that the UI is about to perform, receive keystroke events (think about a plugin that adds new keyboard shortcuts), and everything we want to let a plugin do.

If you think this is a good idea, we can already namespace this MarathonService that we are about to write. We coud have MarathonService.http.request() instead of just MarathonService.request().

Obviously this is a experimental code that would be evolved in future releases. It's also perfectly possible to have two PluginAPIs exposed by the UI, while the old API is correctly deprecated.

[orlandohohmeier]

Thanks for your feedback @daltonmatos!

You're right age doesn't really make sense for requests other than GET and it's probably even unnecessary to cache any plugin requests.

I like the idea of creating a rich MarathonService which provides dedicated methods to create an app or get a list all running apps. I also like the idea of providing a cohesive interface so that plugins don't need to fiddle around with abstract actions and events.

We actually planned to deliver an independently versioned interface but decided to run with an integrated version till we see the need for a separate one. It might be the right time to reactive the repository. What do you think?

https://github.com/mesosphere/marathon-ui-plugin-sdk/tree/feature/pull-3107-plugin-interface

I don't see a need for the http interface in case we introduce a dedicated MarathonService. Why do you think we want this?

How would you like to proceed?

[daltonmatos]

Hello @orlandohohmeier,

All fine about the age parameter. Let's initially forget about it and add it back eventually, if needed.

Being agreed about the implementation of a more cohesive interface (instead of a strictly "raw" implementation) I think we should start writing it as a first iteration of this new plugin interface.

I totally agree about the extracted plugin SDK, but I think that doing it right now would be a bit too much, after all, we are creating something very experimental that we believe will become the new standard for Marathon UI Plugins in the future, so I would think that we should first somewhat confirm that what we are choosing now will work and then we extract the SDK. This will make our lives easier during this early development, and as a bonus if we maintain the same methods, namespaces, etc. when extracting the SDK, no plugin will need to be changed. 😉

About the http interface inside the MarathonService object. I think would be a good ideia to have both a "high level" and a "low level" API in this plugin interface. This will make it possible, for example, for a plugin developer to write a feature that is still not included as a high level method in the Plugin Interface. Think about it as a "I know what I'm doing" feature. 😄

So a plugin could use this "raw" http interface until the Plugin Interface is updated with more suitable methods that helps this plugin achieve what it wants, and then it could switch from the raw to the high level methods.

Another usefulness of this raw API would be to call a plugins' backend http endpoint(s). The UI Core and the exposed Plugin Interface don't know about which endpoints a plugin can or will implement, so having a raw http interface would be useful for plugin developers to call its backend when necessary. Of course that MarathonService could implement a generic way to call a plugin's backend endpoint but maybe this generic way is exactly this raw http api? 🤔

All said, I think we should start writing the MarathonService [1], which will have, for now, just the http namespace, being a direct wrapper for the internal ajaxWrapper. Then we can start writing the dialogs namespace, then the events namespace ans so on until we have all currently exposed interfaces implemented. This will give us a good feedback about this new Plugin Interface, and this could be the time when we start thinking about extracting the so called "Plugin SDK".

What do you think? Give me a green signal and I will start writing this initial wrapper.

What do you think about a new issue on the SDK repo so we can talk about more broad things related to this new interface? We could talk about what will be exposed and how it will be implemented (Plugin mountpoints Vs. Dispatched events, etc). I have some doubts about the code on this branch you linked.

Thanks a lot and sorry for the very long posts. 😔

[1] We can begin with this name and change later, no problem. But we should change it fast as this will be a breaking change.

[daltonmatos]

Hello @orlandohohmeier, any thoughts about this? I was thinking about starting to write some code during the weekend, what do you think?

Thanks,

[orlandohohmeier]

Hi @daltonmatos!

I like the idea of providing a low-level-i-know-what-i-am-doing interface to interact with Marathon. I'd propose the following namespaces on the plugin interface:

• MarathonService – low-level http interface
• MarathonActions – dedicated marathon actions e.g, createApp

I've outlined the respective modules in the following:

MarathonService

This service provides an interface to request Marathon resources. Plugins will need to use the standard XHR or fetch interface to perform arbitrary requests.

ℹ️ This service enables plugins to load files that are served s using the HttpRequestHandlerBase interface. For an exmaple please see: https://github.com/mesosphere/marathon-ui-example-plugin/blob/master/src/main/scala/mesosphere/marathon/example/plugin/ui/HttpRequestHandler.scala

Examples

// fetching groups with defaults
MararthonService.request({
    resouce: '/groups'
})
  .then(function success(response) {},  function error(response) {})


// deploying a new app
MararthonService.request({
    resouce: '/apps'
    data: {new_app},
    method: 'POST'
})
   .then(function success(response) {},  function error(response) {})

MarathonActions

These actions will use the MarathonService to performe the needed requests.

Examples

MarathonActions.getGroups()
   .then(function success(response) {},  function error(response) {})

MarathonActions.createApp().
   .then(function success(response) {},  function error(response) {})

Please let me know if this is sufficient and if you need anything else to get started.

[daltonmatos]

Hello @orlandohohmeier , this is sufficient to begin coding. The only thing I would like to suggest changing (but this doesn't prevent me to start coding) is:

Change the .then(success_callback, error_callback) to the possibility to have both .success() and .error() builder functions.

MarathonActions.getGroups()
   .success(function success(response) {})
   .error(function error(response) {})

I think this approach more pleasant to have. What do you think?

I will try to code something this weekend. Hope I have some free time.

[orlandohohmeier]

@daltonmatos please excuse the late response. I'm fine with have success and error as additional helpers just wanted to make sure we're returning actual Promises to allow chaining if needed.

[daltonmatos]

No problem @orlandohohmeier. I will try to write something and post here. Thanks.

[daltonmatos]

Hello @orlandohohmeier , here is a Proof of Concept about what we talked about.

About the MarathonService.request() method, I opted to not include the /v2 part automatically, since this would make it impossible to request some endpoints outside the /v2 namespace, like /metrics, /ping.

I also decided to have all methods inside MarathonService and MarathonActions as bound methods (not static methods) and as such, PluginAPI needs to instantiate both namespaces before exporting itself.

The same happend when MarathonActions reuses MarathonService.request(). To be honest, I don't know which approach is better.

I think that, if what we are doing works, pluginWindow.marathonPluginInterface will look like something along these lines:

pluginWindow.marathonPluginInterface = Object.freeze({
  pluginId: pluginId,
  PluginAPI: PluginAPI
});

and all other currently exported objects would be migrated to namespaces inside PluginAPI. Doing this we can modify the plugin API without changing any code on the core marathon UI (I'm assuming here that the Plugin SDK already lives in a separated repository).

Let me know what do you think about this. I implemented a sample plugin that uses this new interface and it works as expected.

Thanks a lot.

[orlandohohmeier]

💜 Thanks a lot for putting this together. This is great and it will enable lots of new cool plugins.
I added a few comments to further improve the code and make this as reliable as possible. Once again, thank you.

[orlandohohmeier]

Let's make request static so that you don't need to create an instance of MarathonService.

[daltonmatos]

👍

[orlandohohmeier]

Let's make these "actions" static so that we don't need to create and instance of MarathonActions.

[daltonmatos]

👍

[orlandohohmeier]

I'd add a check to make sure group actually starts with a slash /

[daltonmatos]

👍

[orlandohohmeier]

I wonder if it makes sense to always allow additional option to pass embed flags.

[daltonmatos]

Currently it is possible to pass all needed embeds directly on the resource attribute, like this:

MarathonService.request({
  resource: "/v2/groups?embeds=...&embeds=..."
});

Do you think we need a new attibute specific for this on the object passed to request()?

Maybe this makes more sense on MarathonActions.getGroup(), since there we have a higher level API.

What do you think?

[orlandohohmeier]

Fair point. Let's add these flags to the higher level API.

[orlandohohmeier]

Let's not share the ajaxWrapper.

[orlandohohmeier]

Do we really need to wrap these into a dedicate PluginAPI namespace? Shouldn't it be enough to simple provide MarathonService and MarathonActions?

[daltonmatos]

About the possibility to export all namespaces individually and directly on pluginWindow.marathonPluginInterface or adding the namespaces into a single object and exporting only this object.

The ideas behind what I did are:

• Exporting a single object makes it easier to add new namespaces. The idea is that everything that's inside PluginAPI is available to plugins to use, without any restrictions.
• This makes it possible do add new SDK features without having to modify the code of PluginLoader.js. Think about it when we extract the SDK code to another project. The SDK will evolve independently of the UI. If we want the UI to have a more robust SDK we just need to change the version of the SDK that the UI is using. And this is possible because the interface between the UI and the SDK remains (and will always remain) the same.

This is what I think about what would be a PluginSDK (on a separated repository).

• The core UI code would have this SDK as a normal dependency, saved to package.json, like any other dep. installed like: npm install --save marathon-ui-plugin-sdk
• The code of PluginLoader.js would be changed to:

import PluginAPI from "marathon-ui-plugin-sdk";

• There would be a set of automated tests to check that the current Core UI code is compatible with the current version of the UI. This would be the trickiest part, I think. 😄

With the setup like this, Marathon UI would always be shipped with a specific version of the plugin-sdk. And all this would be done without breaking any plugin code, because the interface is the same, there's only one object for plugins to import: PluginAPI.

Also the plugin code would become simpler:

const {PluginAPI} = window.marathonPluginInterface;

and all further interactions will be done with PluginAPI. Rather than import each namespace individually.

What do you think?

[daltonmatos]

Hello @orlandohohmeier, any thoughts?

Thanks,

[orlandohohmeier]

Definitely not a blocker but you could use destructing to extract resource, method as well as the data as illustrated in the following snippet:

 static request({resource, method="GET", data}) {
    // ... 
    return ajaxWrapper({
         url: `${config.apiURL}${resource}`,
         method: method,
         data: opts.data
       });
  }

[daltonmatos]

This is awesome! Will do it. This will make the code much simpler!

[orlandohohmeier]

I not sure if we need a dedicated util to prepend a slash. Applying a simple RegExp like the following one should do the trick.

resource.replace(/\/?(.*)/,"/$1")

[daltonmatos]

Perfect. Will change to use a simple regex.

[orlandohohmeier]

Let's not create a new util only to add a slash.

[daltonmatos]

Sure!

[orlandohohmeier]

Fair point. Let's add these flags to the higher level API.

[orlandohohmeier]

I'm still don't think we should add a PluginAPI namespace to the marathonPluginInterface. The PluginAPI you're picturing is exactly what marathonPluginInterface is for.
In case we want to move the interface to a different repository I'd extract what we have today and move the entire marathonPluginInterface into an external class so that no one needs to adjust the loader to add a new interface.

Example

     pluginWindow.marathonPluginInterface = new MarathonPluginInterface({
          PluginMountPoints: PluginMountPoints,
          pluginId: pluginId,
          React: React,
          UIVersion: config.version
        });

N.B. The plugin loader probably needs to inject a few dependencies to provide access to the respective Marathon UI bits. We should look into different IOC containers to abstract the the injections to properly decouple the loader and the interface.

[daltonmatos]

About the PluginAPI, what you said makes total sense, I just didn't realize it until I read your message. I will remove PluginAPI and add MarathonService and MarathonActions to pluginWindow.marathonPluginInterface.

[daltonmatos]

All done @orlandohohmeier. Waiting for your review.

[orlandohohmeier]

@daltonmatos this looks great! Thanks a lot for your contribution and efforts to extend the plugin interface to enable new use cases. 🙇