Added Days and Hours for history and daily pulls

[gauthig]

Please review and let me know if this should remain a forked project or merged into your project. The readme is just to acknowledge this work and your original work, it should not be merged, other files would need to be merged. Please provide your comments.

Goals

1. Be able to load all historical emporia data
2. Reset the json file so a second run after using history or RESET did not wipe out previous run
3. Have fast (and low resources) monthly and daily charts while comparing year over year without all the minute data.
4. Next feature will be pull latest min/hour/day on startup to catchup missing data if the program was not running for some time.

Reset config json file values to allow multiple runs without resetting database: Database reset back to False and History Days back to 0 after run.

Add additional scales to the pull allowing to get years of history since minutes/second have a short life at emporia. This also allows faster dashboards for creating items like daily or monthly graphs. Hours history and daily pull Days history and daily pull Notes While adding additional scales will keep seconds = true and minutes = false to not impact existing reporting. Was able to get 720 days of Hours and Days in history load. Day and Hour matched for 2 years of data. Minute/Seconds had slight discrepancy but matched emporia manual csv downloads.

[jertel]

I like the ideas and I appreciate your desire to contribute back to Vuegraf. Here are my thoughts:

• Resetting the JSON values might be a step too far. I never really liked how I implemented the resetDb and load history config settings, but it was simple and accomplished what I was trying to do for this personal project. But as more people use this, I think this needs more thought, as it's not an industry accepted practice to have an app modify the configuration files, nor is it a good idea to require humans to remember to edit the config files themselves, after that initial run. Perhaps instead of making these config settings, they would be more appropriate as command line args. Ex:

  docker run --rm -it -v $(pwd)/vuegraf.json:/opt/vuegraf/vuegraf.json jertel/vuegraf --reset-database --load-history-days=24

• The new and improved history logic you have which can load all history arguably could make the existing load history minutes obsolete. Perhaps that old logic could go away and we instead use the new logic only, when loading history. Realistically, given that per-minute data is short-lived at Emporia, how many users really only want last week's minute data, but not the past 12 months of daily/hourly data? Probably few to none.

• I noticed you have log functions in the getdate.py file. It would be good to separate these into a logging.py file, or use a common Python logger lib.

• The PR will need to update the README.md to reflect your new changes.

• Because this is a bigger change, it probably is a good idea to start a CHANGELOG for this project. I have been using a CHANGELOG pattern in my elastalert2 project with good results so we could copy that to get us started.

What are your thoughts on this feedback?

[gauthig]

Thanks for reviewing.

Resetting the config value are bad, I agree but just wanted to use current process. Yes, I will change and remove both history days / database reset from config to command line parameters. I also prefer this style and leave the config file to operational settings, not initializing.

My Next steps

• Change logic for database reset and history days to be command line options only and remove from json config.
• History load of minutes - Agree, I will make a change to pull that as it does become obsolete.
• Getdate.py - oops, I thought I had it in my ignore file so it would not commit back to the project. That is a future enhancement to automatically load history between last input and new startup. The logging was for debugging only, will not be in final program.
• Readme updates - Yes, I will work on that.
• CHANGELOG - I'll look at it and start using that process.

Since I will remove minutes, do you want to keep Seconds as Detail = True or change to Detailed = Seconds? I'll start next week on the above changes and reject the current pull request.

From: Jason Ertel @.> Sent: Sunday, April 9, 2023 2:55 PM To: jertel/vuegraf @.> Cc: Garrett @.>; Author @.> Subject: Re: [jertel/vuegraf] Added Days and Hours for history and daily pulls (PR #119)

I like the ideas and I appreciate your desire to contribute back to Vuegraf. Here are my thoughts:

• Resetting the JSON values might be a step too far. I never really liked how I implemented the resetDb and load history config settings, but it was simple and accomplished what I was trying to do for this personal project. But as more people use this, I think this needs more thought, as it's not an industry accepted practice to have an app modify the configuration files, nor is it a good idea to require humans to remember to edit the config files themselves, after that initial run. Perhaps instead of making these config settings, they would be more appropriate as command line args. Ex:

docker run --rm -it -v $(pwd)/vuegraf.json:/opt/vuegraf/vuegraf.json jertel/vuegraf --reset-database --load-history-days=24

• The new and improved history logic you have which can load all history arguably could make the existing load history minutes obsolete. Perhaps that old logic could go away and we instead use the new logic only, when loading history. Realistically, given that per-minute data is short-lived at Emporia, how many users really only want last week's minute data, but not the past 12 months of daily/hourly data? Probably few to none.
• I noticed you have log functions in the getdate.py file. It would be good to separate these into a logging.py file, or use a common Python logger lib.
• The PR will need to update the README.md to reflect your new changes.
• Because this is a bigger change, it probably is a good idea to start a CHANGELOG for this project. I have been using a CHANGELOG pattern in my elastalert2 project with good results so we could copy that to get us started.

What are your thoughts on this feedback?

- Reply to this email directly, view it on GitHubhttps://github.com/jertel/vuegraf/pull/119#issuecomment-1501221355, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AOZL7VDD5QQILZE2BMH2OBLXAMV4BANCNFSM6AAAAAAWTFC4Y4. You are receiving this because you authored the thread.Message ID: @.**@.>>

[jertel]

Sounds like a good plan.

You can leave the Detail = True. That live fine-grained detail data will always be in seconds since we already get per-minute data on the regular live data updates. So removing the obsolete historic minute data is unrelated.

[gauthig]

Closing current pull request with pending work listed in the comments.