Revamp Admin Module

[chaveiro]

Fixs or add the follow:

• Full components manager
  -- Add component uninstall
  -- Add component install
  -- Add components available list
  -- Notify if updates available
• added disks read load statistic
• improved disks write load statistic to use (if installed) iostats, remove hardcoded partitions to support non raspberypi devices
• added php native support for components update with git
• added cpu info
• added machine brand model and bios description
• show all services states and added ssh
• Added support for running services from PHP when redis not available.
• Move service-runner trigger run responsibility to the model class runService() function.
• User list view return link
• Use model class the right way: remove static usage and global 'nightmare'
• Support disk write load stats without redis
• Preparation for future script execution / components update without redis
• Remove absolute paths from script files (use config base path instead)
• Uniform code across all modules
• Minor UI fixs
• Fix state machine of update logs when needed so that errors can be seen
• Deal with lack of redis
• Make use of json result codes for all errors across admin views (components, log, firmware and update)
• Move views to ./Views/ folder
• Check symbolic link exists when deleting a module
• Added current user info to main admin page

[chaveiro]

@TrystanLea i consider this work stream finished and all tested, please take a look if some polishing is still needed.
Could you find a way to have the scripts from https://github.com/openenergymonitor/EmonScripts/tree/master/update related to git updates from the admin UI placed at the emoncms/scripts folder (change repo) so that we don't need to install EmonScripts on shared servers ?

Edit: Added support for php to call git natively for the components module so the EmonScripts are not a dependency anymore. If desired to have just a central way to manage updates we can remove the (now optional) way to use scripts from the component. Let me know your thoughts.

[chaveiro]

@TrystanLea please take a look at this screenshot, this is how it looks like with your old (and current code).
Is is supposed to look like that on chrome, above the scroll bar ?

[chaveiro]

Could you give a look at the cpu, machine and read load and write load at the admin page on the raspberry pi?
The slowness you point out is the time the git command takes to run. That can depend on network, etc.

[alexandrecuer]

read and write load :

machine and CPU

[chaveiro]

Thank you, looks ok.

[chaveiro]

@alexandrecuer based on your feedback, added 2ded17f

[TrystanLea]

Hello @chaveiro

First, thanks for making the pull request and suggesting these new changes.

I've started the process of reviewing the code, there are too many changes in a single pull request here to merge as one single merge. I need to separate out the bits I am immediately happy to merge and the other parts I would like to consider more carefully.

So far I am happy to merge as a starting point:

• EmonLogger changes (already merged)
• index.php changes to provide an error if redis is missing (already merged)
• Moving admin views into a Views folder, branch prepared https://github.com/emoncms/emoncms/compare/admin_move_views?expand=1
• Making the admin class non static, branch prepared https://github.com/emoncms/emoncms/compare/admin_move_views...admin_non_static_class?expand=1

This then makes the next set of changes easier for me to see and I will review these next. I cant guarantee that I will be happy to merge everything, e.g Im not sure Im happy yet to merge the changes to the component view to allow the alternative non git approach, I really want to think about that carefully.

Would you be happy if I go ahead and merge the above partial merges as they are with the reference to your pull request and name included in the commit message, or would you prefer to create your own pull requests with the same changes to more perfectly associate the commit with your authorship?

While the practicalities of coding don't always make it easy/convenient for this breaking up of development into smaller pull requests, I certainly find this myself too. It's not too hard to go back after developing a larger body of work and split it up into smaller stages. I use a tool called Meld to do this comparing an unchanged emoncms master with my branch with a lot of changes. I then copy across specific bits to make a smaller pull request that's easier to test and review in isolation.

[chaveiro]

e.g Im not sure Im happy yet to merge the changes to the component view to allow the alternative non git approach,

The PHP approach uses GIT indeed.
Take a look at the model functions, you will see that the way i architected the code keeps the service runner option first if a script file is available and redis + service runner are running.
So to keep old style approach of using shell scripts to call git instead of php functions, you just need to write some shellscripts for the the new functions that were added in PHP (install, uninstall and update reset).

Now, switch to a high level view and try to reason what might make more sense in the long term after the initial installation of git cloning the core emoncms :
a - To write shell scripts to call git in a limited way by passing variables there via command line parameters and using an async call via service runner
b - Call a emoncms documented API for doing the same thing and having only to deal with one code base and one programming language for complex validations of the returning data from git and post install / uninstall executions in PHP.

If you need the async call, you can even call the php functions on the admin API when needed.

From my perspective i think the reasoning should be to get everyting working from the UI in a clean an simple way for the end user to deal with. if it's a raspbery pi, shared server or other. Plenty of success examples follow that: homeassistant, plex, wordpress, just to name a few i've used.

I understand your option to try to focus on only the product with a raspbery. However currently the effort to support 'all' platforms is easy but if you go the route of raspbery pi only, the community that receive future updates and participate on development will be limited. Similar to excluding redis from new features will leave out a partial number of users or getting a different branch for low write we had in the past.

Would you be happy if I go ahead and merge the above partial merges as they are with the reference to your pull request..

Sure i don't mind about the authorship of this, i just want the code to get on main so i can sync back and continue :)

[TrystanLea]

Ok, I've merged the partial merge to master and pushed a copy of your master branch to a branch called https://github.com/emoncms/emoncms/tree/chaveiro_admin_development. I have fixed the conflicts that arose in this branch, you should be able to pull that in hopefully, will get back to you on the above once I get to that part, thanks!

[TrystanLea]

Hello @chaveiro would you be willing to help me merge the above changes by separating out this pull request into smaller stages. I think the easiest way for you to do this would be to use your Beyond Compare tool to create new pull requests that contain one specific set of changes at a time.

A starting example would be a pull request specifically for:

Make use of json result codes for all errors across admin views (components, log, firmware and update)

There's a fair bit of code Id be happy to merge quite quickly if broken down into smaller parts.

I came across this guide for the kubernetes project the other day and it describes really well what I'm after:
https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#smaller-is-better-small-commits-small-pull-requests

I do appreciate this is more time for you.

[chaveiro]

Here we go again.. :)
I pushed individually commits for this with that in mind. If you can follow the commits history it gets easier to understand.
Let me see If i can take a look during the next week, this is full of conflicts now and I'm not sure why.
Please try to look at the code as a whole and do a review. This is very straightforward.

[TrystanLea]

Thanks @chaveiro

I fixed the conflicts in this branch chaveiro_admin_development...chaveiro:master you should be able to pull that into your master branch hopefully and then re-create a pull request to the emoncms master..

I think my request is reasonable, see the kubernetes discussion on this. I could review as a whole but it would take me a good number of days work to do that and it means if there is any part I don't want to merge it will hold up all the other changes.

Thanks for offering to take a look at it

[TrystanLea]

The other option is that I try and go through each commit using git cherry-pick I started trying that way last night: d954f8f but not sure if that's going to create more problems if I make small modifications along the way... I can try it if your then happy to help me with conflicts that might arrise

[TrystanLea]

Using the git cherry-pick tool, I could merge:

• 285499b
• 4c54a8c

Then there's too much happening on this one:

• d337f80

The following one has a lot happening but I think I could work my way through that one and merge:

54156e5

Not sure what happens if I skip d337f80 and merge 54156e5 feels like things could get messy.

The first two commits are great, small and concise, easy to merge, it then gets a lot harder and I struggle to follow

[TrystanLea]

Which is why I think it might work better if you create new pull requests that pull out specific changes one at a time. Each pull request would do one thing at a time e.g Make use of json result codes for all errors across admin views (components, log, firmware and update) and would not contain any other changes that do not relate to that change e.g: refactoring, css/layout changes etc.

[TrystanLea]

Ok d337f80 is a git chery-pick skippable commit. So lets see if I can do the next

[TrystanLea]

Ok:

• fbea391
• 55fa35d
• 913629e

have now been merged,

The branch that I mentioned above that I had fixed conflicts in is now full of conflicts again when comparing with master so may be best to ignore that branch now, will keep going with cherry-pick see were I get too

[TrystanLea]

A couple more commits merged to master:

• 67fc05a
• ae755eb
• e20fb59
• 3dd37aa

[TrystanLea]

And some more merged:

• 3a50643
• f232233
• 8befed9
• a9bd2e8
• 3293bd5
• a7ed96f
• 813a85c

I will keep on with this approach, will let you know when I have finished and if I need further input at that stage. It will probably not be today.

[TrystanLea]

More commits merged:

• 813a85c
• 484f936
• cd71480
• dc2fbab
• 909dca4

The remaining commits are all to do with the component manager which I would like to review more carefully as this was a part of the code I had intended to work on myself. I will try and come back to that when I next get some time hopefully later next week.

Not sure if its worth you rebasing your master branch on main now or waiting until I've reviewed the remaining commits.

[chaveiro]

Hi @trystan, I've tried to look at this and got a little confused on what is currently the status.
May i suggest to do it like previously you do a code review and i answer and change behaviors to your preference?

[TrystanLea]

Sure, I've merged most of the commits up to those affecting the component manager. I need to get through some other work before I get back to this, so it will probably be a couple of weeks before I can continue the review.

If you are ok to wait with this pull request linked to your master branch then feel free to do that, but if you did want to free up your master branch to pull in upstream changes, then it might be worth creating a new branch with the remaining commits that are yet to be merged inside.

If you create two branches one that just holds your current master branch status e.g:

git checkout -b master_backup

Then do another to test merging in the current upstream master:

git checkout -b master_merge_test
git fetch upstream
git merge upstream:master

If you think you can fix the conflicts that result then give that a go. If you get stuck you can always delete your merge test e.g:

git checkout master_backup
git branch -D master_merge_test

If you do manage to fix the conflicts it might be worth trying to open a pull request from your master_merge_test branch against the upstream emoncms master.

In hindsight I should have avoided the initial manual merge that I did moving the view file locations to make the git diff easier and just followed the git cherry-pick process above from the start. I will try that first next time.

[TrystanLea]

Hello @chaveiro

I've fixed the merge conflicts resulting from my partial merge and pushed the result to a new branch here #1765 from which I will continue the review process. You should be able to pull in from that branch to updated your master branch to include all the other recent commits to emoncms master if you want. Il create a pull request to your master branch.

Closing this pull request to work from #1765

Thanks