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

Python3-3.10 some quick fixes for some Quality of Life Issues #39

Conversation

trwbox
Copy link

@trwbox trwbox commented Jun 8, 2023

These fixes were all made and tested on the Python3-3.10 branch, but I have no reason to believe the same ideas can't be applied to the Python3-3.8 branch as well. I just do not have a system setup with that I can test them on.

Changes:

  • Create the Logs directory if it does not already exist when starting the program, to prevent the logger from erroring out. This should also fix issues where if the logging filepath is changed in the YAML config, the directory and file will be created properly. (There are still some hardcoded path in the Log tab of the GUI that I want to look into getting changed next, to follow the path set in the YAML).
  • There are some locations where a float is wanted in a text box, but putting in white-space characters like tab, would result in an escaped string 433\\t being passed to the next program, which fails to parse as a float, so added stripping to prevent this issue (Note: This likely could have some more checks done that is actually converts to a valid float)
  • For some textboxes that were requesting a port number, they were not verifying that an valid int/port number were passed, so adding a check that it is a valid integer and port number.
  • Added some really common files to the .gitignore, as a method to preemptively prevent them from ending up in the repo.
  • Added a super basic signal handler, so events like CTRL-C from the command line will cause a graceful closing instead of the helper python scripts getting left open and keeping sockets open. I believe that it treats it just as X on the GUI would currently.
  • Added some checks when attempting to launch command line programs that are not part of the minimum install, so that an error is given to the user instead of attempting to run a non-existent program, and throw a "silent" error in the log.

Most of these may occur more times in the program, these are ones I happened to run into while poking around, and could get narrowed down quickly.

@cpoore1
Copy link
Collaborator

cpoore1 commented Jun 10, 2023

I will investigate these points with more detail in a bit. Here are my initial thoughts:

  1. The Logs directory should always be there. If the goal is to move it somewhere else, test pieces without it, or rename it, then most of those changes could be done. The logging.yaml file does include the Logs/event.log location and the dashboard.py code points to the Logs/event.log file without pulling it from any yaml/configuration file in a couple places. The logging mechanism needs updating in a bunch of areas.
  2. I will look into applying validators on a broader scale for the user inputs.
  3. I definitely will insert checks to only accept valid port ranges.
  4. I have no problem with adding common troublesome extensions to .gitignore. Although, I'm curious if anyone is editing this or including secondary files while on MacOS.
  5. I'll test out the graceful closing. That really needs to be fixed to prevent annoying kill/pkill commands on crashes. Plus the killing of the other programs can cause a select number of people to log out. It is complicated a little by the fact that the components were designed to be run on different computers across a network. I have not tested each component away from localhost in a long time and that should be looked at again.
  6. There are essentially no checks in place if menu items are not installed and accessed within the menu. Those will be added where possible going forward.

@trwbox trwbox closed this Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants