Skip to content
This repository was archived by the owner on May 12, 2021. It is now read-only.

Adding RCMES cli code#154

Closed
MBoustani wants to merge 1 commit intoapache:masterfrom
MBoustani:Adding_RCMES_CLI_code
Closed

Adding RCMES cli code#154
MBoustani wants to merge 1 commit intoapache:masterfrom
MBoustani:Adding_RCMES_CLI_code

Conversation

@MBoustani
Copy link
Copy Markdown
Contributor

No description provided.

@OCWJenkins
Copy link
Copy Markdown

Merged build triggered. Test Failed.

@OCWJenkins
Copy link
Copy Markdown

Merged build started. Test Failed.

@OCWJenkins
Copy link
Copy Markdown

Merged build finished. Test Passed.

@MJJoyce
Copy link
Copy Markdown
Member

MJJoyce commented Feb 26, 2015

Should really remove all reference to JPL/NASA/RCMES since this isn't the RCMES project. Also, why is it called the ocw-cli but in an RCMES-cli folder? Why not just add to the existing tool that we already have? Or even just wholesale replace if it's not sufficient?

+1 to merge once the above are addressed. Looks much more flushed out than the current ocw-cli at first glance. Nice stuff.

@MBoustani
Copy link
Copy Markdown
Contributor Author

Mike it should call RCMEC-cli.py my mistake, I will change the name in next PR.
I think we need to remove OCW-cli folder and OCW-cli.py and have a folder called RCMES-cli which has file called RCMES-cli.py . This will represent one example/tool made from OCW library for RCMES project.

@kwhitehall
Copy link
Copy Markdown
Contributor

hrm. @MBoustani I definitely agree that the RCMES-cli.py should go into its own folder as it is an application of OCW. But, I'm not entirely sure about removing the ocw-cli and ocw-cl.py Actually, in fact, maybe it should reside in a branch, and remove the ocw-cli, etc. there....

@MBoustani
Copy link
Copy Markdown
Contributor Author

@kwhitehall what is ocw-cli.py right now ? It is an old version of RCMES-cli.py, I have just modified the OCW-cli.py and changed the name to RCMES-cli.py as it should be. So there is no point to have an old broken OCW-cli.py in repo, and instead we should have RCMES-cli.py that used OCW as backend to run some evaluations that have been designed in RCMES project.

@kwhitehall
Copy link
Copy Markdown
Contributor

True @MBoustani I'm thinking about later down. As functionality for e.g. is added to OCW, persons may wish to change the cli or update it etc. Whilst the RCMES project may decide to wait or not include that functionality. So that's why I suggested a branch.

@MJJoyce
Copy link
Copy Markdown
Member

MJJoyce commented Mar 4, 2015

-1 on calling this the rcmes-cli. @MBoustani, you wrote the original CLI. If it's broken and this fixes it, then we should commit this as a fix to it and change the RCMES references to OCW. There's quite literally one thing to change:

TITLE = "Regional Climate Model Evaluation System (RCMES)"

I don't see why we would brand this the RCMES CLI when it seems like it's simply fixing the OCW CLI, which is functionality that we want to have in the toolkit. There's nothing here that is inherently "RCMES" that I can see.

@MBoustani
Copy link
Copy Markdown
Contributor Author

@MJJoyce the reason we should call it RCMES and not OCW because all the recipes to designing this CLI came from RCMES project and we just used OCW as backend. There is no evaluation step has been defined as it is in RCMES project. RCMES project just uses OCW as backend and it is a GREAT tool/example made from OCW.
Think about GISCube project that I made and I have used GDAL at backend of it, so that does not necessary means I should not have call it GISCube and call it GDAL..!!
Let's have @chrismattmann and @darth-pr in the loop as well.

@MJJoyce
Copy link
Copy Markdown
Member

MJJoyce commented Mar 6, 2015

Quote from you

So there is no point to have an old broken OCW-cli.py in repo, and instead we should have RCMES-cli.py that used OCW as backend

Yes, and we should call it the ocw-cli, not the rcmes-cli.

@chrismattmann
Copy link
Copy Markdown
Contributor

Why can't this just be an "app" that runs on top of OCW. We have examples of this all the time. For example, in Apache OODT, we have apps that reference config that were developed in projects at NASA JPL (for example PGE config originally developed on the JPL Airborne Snow Observatory project; workflow policy originally developed on NPP, etc.)

These example apps and config are simply that - example apps and config. It seems the core of the debate here is whether or not the command line interface here is an "app" that runs on top of OCW; or whether it's a core part of OCW.

I really have no preference here per se, but can see both sides of the coin. I don't think this is a blocker issue. One way could simply be to make those two properties at the end (organization and title) configurable? If they were configurable (e.g., set by an environment variable and property file, with sensible defaults set, even to JPL and RCMES in question), then I think it's fine.

@MJJoyce
Copy link
Copy Markdown
Member

MJJoyce commented Mar 11, 2015

+1 Chris. The only reason I was bringing this up is that this is seems to be a fix to code that @MBoustani already committed to the code base. I'm fine with this being the only CLI example in the code base but the only thing making this something that is part of "RCMES" instead of a generic OCW CLI is ~1 line (see above). This was the same logic that was applied the UI. There were actually far more changes to the UI in order to make that generic for OCW instead of being branded.

If we are going to commit this as the RCMES CLI that is an example tool built on top of OCW then I think it's beneficial to discuss some small changes to the repository structure. I don't think top level folders should be for outside example use-cases that aren't supported by the project and it needs to be clear (in a README or otherwise) that questions on how to use it go to the appropriate mailing lists. At one point someone brought up the idea of an Examples-esque folder for stuff like this to be put in (maybe it was @kwhitehall?) and I think that's a good idea.

All that being said, my +1 goes to stripping out the small amount of config that brands this RCMES and making this the new OCW CLI.

@kwhitehall
Copy link
Copy Markdown
Contributor

Yip @MJJoyce that was my suggestion w.r.t. the examples-esque folder or apps-esque folder, and I think this thread drives that home.

So I gather that:

  1. we all agree RCMES is an app of OCW and the RCMES-cli should go into the repo as is, in its own folder or something (hence a discussion of the repo structure is necessary)
  2. OCW-cli should be updated to ensure that it is not broken

I am still on the fence w.r.t. simply rebranding the RCMES-cli to OCW-cli to accommodate 2 above. Firstly, if we decide to go this way for this PR, we don't want to set precedence. After all, if we all agree the workflow and functionality of the RCMES-cli is ONE application of using the OCW libraries then (1) Do we want the OCW-cli to demonstrate only that workflow? (2) Also, should we then consider the OCW-cli to be an example of how to use OCW (i.e. place it in the examples folder) if RCMES-cli is considered an app of it?

@MJJoyce
Copy link
Copy Markdown
Member

MJJoyce commented Mar 11, 2015

@kwhitehall with regards to your last sentence. I think the biggest difference is that the ocw-cli is functionality built on top of the OCW library that the project supports and actively promotes. This is similar to the ocw-vm and ui. It's tools for exploiting the library in ways that some users might find useful. Those make sense to be top level folders in my opinion although others may disagree. However, if this was committed as the rcmes-cli then it's simply being added as an example of how someone used OCW. It's functionality that is built on top of the project but in no way is it supported or endorsed by the project. I think a very obvious separation is important because of that.

Truthfully, I'm not even a fan of having something like that in the code base but I realize that differs from the opinions of many on the project. It seems to me that if there's not going to be support for (and active development on) the tool from the project then it has no place in the project's repo. It just opens the door for confusion. For instance, we shouldn't be opening tickets for issues on this code. Active development shouldn't be going through the ASF repo for some external companies example code right? Any user questions should be directed to the company that actually supports the code not us. When/how will the code be updated when future API changes inevitably break functionality? I would say that linking to official release from an external company that wants to provide an example of functionality would be ideal.

@chrismattmann
Copy link
Copy Markdown
Contributor

@MJJoyce your statement about "endorsed" by the project has to do with endorsement by the PMC. The PMC doesn't endorse anything here at the ASF. They do "sign" releases, and vet them, but it's very much driven by someone who makes release as the RM, and then a VOTE of the PMC in which there are more +1s than -1s, etc.

People scratch their itches in open source code all the time. @MBoustani is a PMC member of the project here, and he has scratched an itch by developing this CLI which to date, besides the UI, is the only example that we really have of not having to know Python or to be a Python programmer to take advantage of the stuff provided by OCW.

Your comment about not calling this RCMES, b/c this project is called Apache OCW, is a fine one to make, but not considering the fact that in the Apache OCW proposal it is clear that the code was seeded by RCMES as a project and there is provenance there. We don't want to cause confusion and if this were a consumer or business/industry market, then we might about naming and branding from a product perspective. But we're talking about code and a name from the progenitors of the project which involves not really competition, but people that are also working on the code at JPL too.

If @MBoustani thinks RCMES is a good name to call this, and he has developed a neat app that illustrates OCW, and we have right now RCMES as pretty much the flagship application that uses OCW why wouldn't we want to show an affinity and affiliation here between the names?

I think technically the way to get it done (so that other institutions can use OCW and call it UCLA Climate Power Tool; or IITM's Super Climate Diagnoster, etc.) is to make those two parameters in the CLI, institution and project name, configurable as env vars. The default values for those can be set to OCW and Apache; then downstream, we can easily have a quick .properties or Python .ini file that sets this to NASA JPL and RCMES in that case. I think that would satisfy everything.

@MJJoyce
Copy link
Copy Markdown
Member

MJJoyce commented Mar 12, 2015

@chrismattmann thank you for that last paragraph. You've said exactly what I've been trying to say without being nearly as confusing as I'm being =D

@MBoustani First off. This is great and thank you for putting it up in a PR. My point above regarding naming was that this is a great tool that I think other people will want to use. It's a great way to get people to use OCW without needing to write Python. While having this as a RCMES branded example of the toolkit is great, I feel like it's far more powerful to have this be listed as a generic OCW tool that is configurable for anyone to use. So we can host it at JPL and call it the RCMES CLI and we can host it at UCLA JIFRESSE and call it the UCLA CLI, etc. That was really what I was trying to get across with the naming concerns. Let's not make this seem like JPL's tool that is an example of the toolkit being used. Instead, let's make this the generic toolkit CLI that anyone can use and that is can easily be configured for your organization to use as well. Hopefully that clears up the point I was trying make (admittedly I did a poor job).

+1 to @chrismattmann's recommendation regarding making the settings configurable. I think that's the perfect solution.

@kwhitehall
Copy link
Copy Markdown
Contributor

+1 to @chrismattmann suggestion to make the settings configurable - that solution perfectly crosses the t's and dots i's in this PR and for future similar scenarios.

@MBoustani
Copy link
Copy Markdown
Contributor Author

Thanks @chrismattmann for finding a good solution for this disagreement, I totally like the idea of having an "app" folder (in any place in OCW level that fits perfectly) and put the cli as CLI.py along with a config file that holds title and organization name.
I will close this PR and open a new one with new structure and latest version of code.

@MBoustani MBoustani closed this Mar 13, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants