Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Break out the version to a separate file. #745

Closed
wants to merge 7 commits into from
Closed

Conversation

pb66
Copy link
Collaborator

@pb66 pb66 commented Nov 21, 2017

Although I do not think "module.json" is the right name for this file we need consistency across all repos so that we can create a standardized update method. Ideally we need the index.php to read this file rather than have the index.php file updated at every new tag/version.

Although I do not think "module.json" is the right name for this file we need consistency across all repos so that we can create a standardized update method. Ideally we need the index.php to read this file rather than have the index.php file updated at every new tag/version.
@pb66
Copy link
Collaborator Author

pb66 commented Nov 21, 2017

I also think this might be a good place for the $config_file_version = "10" settings.php file version info to live perhaps?

Since the "module.json" has not yet rolled out to all modules, we could consider changing the name to "version.json" so it is universal across all repos including firmware and emonSD's etc etc

@pb66
Copy link
Collaborator Author

pb66 commented Nov 22, 2017

I've been working on an automated updater that uses the "modul.json" files we have discussed and seen in the app and dashboard modules.

Here I propose the samething but for the core emoncms repo too. I would like to see all the files use the same name so I question whether "module.json" should be revised to "version.json" before it is too widely rolled out.

I'm happy to do the PR's including the minor edits in the admin module to make the switch.

Over the last week of so we have seen 10 revisions to the index.php due to updating the version details, a separate file will result in less commit history on the index.php file and users can make local changes (as I have) without having to rewrite those changes at every update regardless of whether there is any real change to the index.php.

Any thoughts? @TrystanLea @Paul-Reed @glynhudson @chaveiro

@Paul-Reed
Copy link
Member

Not sure what the benefits are to now calling it version.json instead of module.json, but no objections here, so long as there is consistency.
The name 'module.json' was the preference of @glynhudson so maybe his view maybe stronger than mine.

Paul

@glynhudson
Copy link
Member

Hi @pb66 ,

Good idea. I agree the version number would be better in it's own file

As to the naming conversion module.json vs version.json I have no particular preference. I guess 'version' would maybe be more intuitive when used on Emoncms core.

My original idea for module.json is that is could be extended in the future to include more info about each module e.g. name, description, dependencies etc.

Could we not use version.json for Emoncms core and module.json for the modules? Maybe they could be interchangeable? I.e the same process that reads and processes the file the would honour either filename?

@pb66
Copy link
Collaborator Author

pb66 commented Nov 23, 2017

DO NOT MERGE !!!
There seems to be an unexpected side effect to logging in that needs investigating.
sorted.

"Not sure what the benefits are to now calling it version.json instead of module.json"

as Glyn says

"I guess 'version' would maybe be more intuitive when used on Emoncms core."

that was my thinking too

"My original idea for module.json is that is could be extended in the future to include more info about each module e.g. name, description, dependencies etc."

I understood that, but when I originally suggested the "version file" and it's use on the admin page, I had a more universal idea in mind, broader than just emoncms modules.

There is no reason why "the file" cannot hold more info about the modules like you wish or other info about other parts of the softwares, the one thing that is consistent across all software is versioning, for an intuitive file name for "the file" that always holds the version details and sometimes some other info depending on the type of software, is IMO "version".

"Could we not use version.json for Emoncms core and module.json for the modules?"

I think that's messy and bloating any updating script to either test for all the possible file names or to make a decision based on what type of software is being updated. How would it tell?

I am making the updating utility pretty generic so it can also be used for firmware and other softwares such as emonhub, perhaps it might even get considered for the emonSD/ emonPi.

I would like it to check for the same file on all repos, if there was any need for the updater to know what type of software it was updating, this is also the place for it to look.

info.json
version.json
version.info
details.json
version.txt
etc

are all files names that are generic to any repo/software and also to the data they hold, I just think that "module.json" is too specific to the emoncms modules.

The alternative I guess, is to use completely different files, I would have preferred a plain text file with just the version alone in it and would be happy to follow that route if you want a emoncms module specific file. I have worked to accommodate using json instead so we have one file but a separate txt file would be easier for me.

@glynhudson
Copy link
Member

I think that's messy and bloating any updating script to either test for all the possible file names or to make a decision based on what type of software is being updated. How would it tell?

Ok sure, let's go for switching to version.json. It's still quite early days and module.json has hardly been adopted further than the repos we have control over. As you said earlier it would not be difficult to change.

I am making the updating utility pretty generic so it can also be used for firmware and other software such as emonhub, perhaps it might even get considered for the emonSD/ emonPi.

Great, yes I will certainly consider it. The emonPi update is currently nice and simple but does have it's limitations.

The alternative I guess, is to use completely different files, I would have preferred a plain text file with just the version alone in it and would be happy to follow that route if you want a emoncms module specific file. I have worked to accommodate using json instead so we have one file but a separate txt file would be easier for me.

I think using json has it's advantages to be able to encapsulate more information. Since we already have a working solution using json let's stick with json, it's the chosen solution for many platforms e.g. package.json for node-RED nodes.

@pb66
Copy link
Collaborator Author

pb66 commented Nov 30, 2017

Thanks @glynhudson

I'm happy to use json if we are doing this in one file which makes sense, my point was only if we had separate files for info and version, there was no need for json for one value, a text file would have been simpler.

Obviously for this updater to work on the data in the version.json files, they must be kept updated so we must discipline ourselves to ensure the files are kept updated in all the repos, I have also built in a "force" feature which will ignore any versioning and update all repo's regardless, this will allow devs (and users?) to pull in minor changes between version releases, but we don't want to fall back on using that as the norm.

I will aim to make the changes over the weekend, I will not merge this PR as it is out of date and may introduce merge conflicts. I will use the correct version and date at the time.

Hopefully @TrystanLea will get a chance to comment before then as I would be happier if we were all on-board, I do not want to push any thing we are not all happy with.

@pb66 pb66 requested a review from TrystanLea November 30, 2017 10:59
@glynhudson
Copy link
Member

Sounds good. I have asked @TrystanLea to take a look and comment.

@pb66
Copy link
Collaborator Author

pb66 commented Dec 5, 2017

I've hit a bit of a hurdle with the "2 part" emoncms modules such as emailreport and postprocess etc.

Currently these modules are installed elsewhere and a sub-folder gets symlinked from emoncms/Modules, The modules.json is located in the folder that gets symlinked which is easy for emoncms to find for populating the admin page.

Any version file needs to be located in the root directory of the module for easy location, both locally and online at the GitHub repo. I have toyed with several solutions and find that for the version.modules data file to remain as one file there is no way around this as we cannot look for a module specific path.

I had already started installing these modules in a specific folder (eg /home/pi/emoncms_modules ) to allow the same "update all in folder" approach without unintentionally updating other git repos that happen to also be installed in the home_dir, prior to now I considered this good practice and found it a much tidier approach to just installing everything to the home dir.

With this 2nd Modules folder it would be pretty straight forward to make the admin page look in 2 locations (emoncms/Modules and emoncms_modules) for installed modules, this would allow other repo's to be included with emoncms even though they are no strictly "emoncms modules" eg emoncms/useful_scripts.

The path to the 2nd folder is already catered for in the settings.php https://github.com/emoncms/emoncms/blob/master/default.settings.php#L71-L72

    // For use with emoncms modules that require installation in an alternative directory to emoncms/Modules
    // $homedir = "/home/username/emoncms_modules";

although perhaps a change of name to something more appropriate $remotedir, $securedir, $alternativedir or $nonwwwdir etc, since it needn't even be in the home dir.

The alternative is to turn things on their head and put the "non-www" folder inside the modules folder, install to the emoncms/Modules folder and then move/symlink it out so the version/module file is in the root and remains there for the emoncms/admin page. But I expect this is a security risk if users do not move those sub-folders, not to mention the headache of changing everything.

So the options seem to be
1 separate version and module files
2 put the files at the root and get emoncms/admin to check 2 folders

there was a 3rd option of symlinking ./version.json to ./www_sub_folder/version.json inside the module so that the symlinked sub-folder also appears to have a copy of version.json, but IMO this is getting messy and only works on Linux.

Any objection to the admin page looking in 2 locations? I already have my installer and updater scripts doing this and there is no escaping emoncms needs a folder outside the www path.

@Paul-Reed
Copy link
Member

Paul-Reed commented Dec 5, 2017

Rather than creating alternative locations, wouldn't it make for a cleaner solution for the emailreport and postprocess modules to be updated so that they can be installed and run from within Modules, so that there is some commonality with the other modules?

Paul

@pb66
Copy link
Collaborator Author

pb66 commented Dec 5, 2017

That would be cleaner yes, but I think these modules have been designed this way specifically to keep parts of it out of the www path so they are not exposed to the network.

I'm not suggesting adding another location, just tidying up and formalizing the fact we already have more than 1 location, to 2 specific and managed locations, and then extending the admin page's module search to include that fact.

@Paul-Reed
Copy link
Member

Paul-Reed commented Dec 5, 2017

Yes, I get that, but to have modules/data scattered around the OS is not a good design principle, and adding an additional function to pull things together seems like using a sticky-plaster to stop the hemorrhage.
Node-red, by example contains all modules, data & credentials in just one directory, so making backups, restorations and problem solving so much easier.
We should be moving towards that model.

@pb66
Copy link
Collaborator Author

pb66 commented Dec 5, 2017

"but to have modules/data scattered around the OS is not a good design principle,"
Absolutely, I couldn't agree more, hence my suggestion to round up all those that cannot go in emoncms/Modules to one (other) folder. I'm not informed enough to challenge the need for 2 locations, just that IF they cannot all reside in one folder, it should be restricted to 2 and both should be looked at by the admin page.

I'm all for moving towards a single location but I don't think that's going to fly, even if possible, there will be little or no appetite to change what we have. In the meantime...

I can most likely implement a change to search in 2 places myself, where as restructuring several modules and ensure security is beyond me. I do not see this a "sticking plaster" as it is simply encompassing the 2 types of module (network exposed and non-network exposed), if in time they can be merged, great we'll only have one location. but there have been modules that cannot be installed to emoncms/Modules for many years so I do not expect that to change overnight, but who knows?

@Paul-Reed
Copy link
Member

I'm all for moving towards a single location but I don't think that's going to fly, even if possible, there will be little or no appetite to change what we have. In the meantime...
@glynhudson @TrystanLea your thoughts please.

Paul

@glynhudson
Copy link
Member

Why not just keep things simple and always place the module.json version.json file in the the root of the folder to be symlinked into var/www/emoncms/Modules? e.g. for the emailreport module the .version.json file would reside here in the emailreport/emailreport-module folder. This way the file is in the correct place (as far as emoncms is concerned), I don't think its too onerous for the developer to enter this folder to view or make a change to the version.json file.

Yes, security is the reason we have 'two-part' modules. It's more secure not to keep certain files out of the web-directory.

I agree it would be a good idea to standardise where these files are kept. Having a standard folder e.g. ~/.emoncms-modules would be a good idea. We could just specify this 'recommended' path in the module install guide, e.g. we currently recomend for the emailreport

ln -s /home/user/emailreport/emailreport-module /var/www/emoncms/Modules/emailreport

this recommended install path could be changed to

ln -s /home/user/.emoncms-modules/emailreport/emailreport-module /var/www/emoncms/Modules/emailreport

Any thoughts @TrystanLea ?

@Paul-Reed
Copy link
Member

Should this 'standard' folder also include the emoncms data files too, as well as the settings files?

This would make the backup/restore process easier, and in many respects replicate the model used by node-red.
Node-red runs a test at start-up, to find out if the settings file exist in /home/pi/.node-red and if it does, then node-red uses those settings & paths. If not, it uses a default settings file.
Emoncms could run a similar test, and also include the path for emonpi's.

/home/pi/.emoncms
/home/pi/.emoncms/Modules/ (or should that be /home/pi/.emoncms/modules/)
/home/pi/.emoncms/settings/
/home/pi/.emoncms/data/

Paul

@borpin
Copy link
Contributor

borpin commented Jan 28, 2018

How would it manage non-standard installations?

@Paul-Reed
Copy link
Member

Paul-Reed commented Jan 28, 2018

How would it manage non-standard installations?

In what way, can you give an example?
If the settings file did not exist in /home/pi/.emoncms/settings/ or the other test locations, it would fall back to /var/www/emoncms/ where users could specify bespoke locations.
The 'data files & modules' absolute or relative paths would be specified in the respective settings file.
ie 'datadir' => '../data/phpfiwa/'

Paul

@borpin
Copy link
Contributor

borpin commented Jan 28, 2018

DietPi does not use /home/pi. Windows installations?

Not suggesting it won't, but I do worry that there is a risk of being sucked into everything revolving around the RaspberryPi and Rasbian. I do think it is a good idea though.

As there is no 'install' script for Emoncms (unlike Node-Red), this folder path cannot be set at install time.

it would fall back...

I'd suggest the opposite in that, instead of falling back to the install folder, the data directory location is set within the install folder (could include it in the administration page). There is of course already some path data within the settings.php file.

@Paul-Reed
Copy link
Member

Paul-Reed commented Jan 28, 2018

...but I do worry that there is a risk of being sucked into everything revolving around the RaspberryPi and Rasbian.
As I said above "or the other test locations", those 'tests' could cover all of the common OS, diet-pi included.

As there is no 'install' script for Emoncms (unlike Node-Red), this folder path cannot be set at install time
Agree, but then presently the data directory structure is manually created. It could be got around by using a script.

...the data directory location is set within the install folder
As I said above, the data dir's paths really needs to be specified in the settings file (wherever that may be), so if the 'standard' folder is backed up/restored, we would not then be having to edit other files to restore paths.

@TrystanLea
Copy link
Member

Happy with original pull request here and the use of the version.json file name. Can we separate out the wider question about module locations for now? and place the version.json in the public_html folder for the emailreport and postprocessing modules.

Then come back to the wider question in our discussion here https://community.openenergymonitor.org/t/move-data-settings-file-to-a-common-location/6464/19

@TrystanLea
Copy link
Member

or should we come back to this pull request after sorting out the latter?

@pb66
Copy link
Collaborator Author

pb66 commented Feb 23, 2018

"and place the version.json in the public_html folder for the emailreport and postprocessing modules."

I am trying to write a generic updater that is not specific to emoncms modules, for what I am trying to achieve the version MUST be in the root of the folder/module, we can symlink it later, but the updater needs to find a version in the root of the on-line repo to establish if an update is available, I'm not happy with the path changing for different modules or repos, many repos are not even going to have a "public_html" folder, that is generic to certain emoncms modules (usefulscripts doesn't have one) and none of the firmware repos or the emonpi or the emonhub repos will have "public_html" folders.

Serving "PHP" from a "public_html" folder isn't entirely clear and I can see that getting changed along the road, we could end up with a string of places to check for the version file and that not only adds extra code and extra processing etc, it also adds confusion and opens the door to issues arising from multiple version files.

Having the version at https://github.com///blob//version for all repos makes for a very reliable and simple system.

@TrystanLea TrystanLea requested a review from emrysr July 10, 2018 17:27
@TrystanLea
Copy link
Member

I realise this is very similar to the pull request I have just merged from @emrysr.

Can you share your automated updater @pb66 that might help with understanding what your trying to do?

@TrystanLea TrystanLea closed this Oct 2, 2018
@TrystanLea TrystanLea deleted the pb66-patch-1 branch February 28, 2019 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants