acmeair / acmeair-nodejs

A Node.js implementation of the Acme Air Sample Application. With datastore support of MongoDB, Cloudant, Cassandra. With runtime support of Bluemix/CloudFoundry, Docker... With Micro-Services.
Apache License 2.0
109 stars 74 forks source link

Add Configuration Page to NodeJS AcmeAir #2

Closed kjschnei001 closed 8 years ago

kjschnei001 commented 9 years ago

These changes add configuration viewing and web-based database loading behavior to Acmeair NodeJS that will add functionality that is already provided in the Java version of the application. acmeairconfig

yanglei99 commented 9 years ago

I would like to see an approach that Configuration is a separate application which delegates to NodeJS or Java application's REST API for the needed information. This way we can maintain one version of Configuration application across by any runtime (currently Java and NodeJS) , w/o duplicating the UI part in individual languages.

kjschnei001 commented 9 years ago

After thinking about this some more, I do agree that we need a long term solution for de-duplicating the front-end UI code for both AcmeAir applications (Java and NodeJS), but I'm not convinced that a separate application is the way to go, particularly for the configuration page. My thought is that this page will be used at least in part as a measure of debugging an environment, and separating it into another application but hinder any attempt in doing exactly that.

I do like the idea of separating out the UI code (all of it and not just the configuration UI), but I'm wondering if this is something better suited as a build time task. I could see merit to integrating the client code at various times throughout the application life cycle. Either way, I think the front-end / back-end separation changes are outside the scope of this change set. In fact, this change makes the NodeJS application closer to functionally equivalent with the Java version and should make such a separation task easier in the future.

dougrt commented 9 years ago

I agree that it would be good to have the UI decoupled from the application implementation, so that the UI can be kept common between all implementations (Java, Node.js, or other future implementations..).

And I also see the point with keeping the Configuration/Administration functions separate from the application's business functions. However at this point in time, I don't know that there are enough of those configuration options that it warrants a whole separate application.

I see this change as a step in the direction of coming to a common UI between the Java and node.js implementations and at the same time adding some additional functionality that is nice from an administrative standpoint. It is also moving the "Load Database" link from the index.html page to a configuration page. I believe this makes sense as Loading the database is not one of the business methods of the application so it should be removed from the application UI and put as part of the administrative tasks.

I would advocate for accepting these changes. And then after merging, we could discuss the best approach of maintaining a common UI for all implementations going forward.

kjschnei001 commented 8 years ago

So is everyone okay with me committing the change?