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

Allow overriding config file, database, and other paths from the command line #20

Open
hawkw opened this issue Feb 7, 2024 · 21 comments
Labels
enhancement New feature or request Stale

Comments

@hawkw
Copy link

hawkw commented Feb 7, 2024

Currently, when running the server, the config file (config.yaml), must be in the same directory as server.py, the database (vudials.db) and uploaded image files are always created in that directory, and the logger always logs to /home/${USER}/vudials.log.

I'm running the VU-Server as a systemd service on Linux, and I'd like to be able to put the VU-Server code in /usr/bin/. The convention on most Unix-like operating systems (the Filesystem Hierarchy Standard, or "FHS") is that /usr/bin contains only executables, and the configuration files and service state files are stored in other locations (such as /etc/opt for configuration files, /var/lib for state files like the database, and /var/lock for the PID lock).

It would be really nice if there was a way to configure the various directory paths used by VU-Server, rather than always using the same directory in which the Python code for the server libs. Perhaps we could add command-line arguments to the VU-Server application, to allow users to override these paths, while retaining the current behavior as a default when the paths are not overridden. Alternatively, the server could detect the operating system in use, and use the FHS conventions when on Unix OSes such as Linux. Potentially, this could be in addition to command-line arguments for configuring the paths. My assumption is that the Windows executable produced by pyinstaller relies on everything being in the same directory, but perhaps not --- if that's not the case, I imagine there are similar conventions for the use of the AppData\ directory on Windows that the server also ought to respect?

@hawkw
Copy link
Author

hawkw commented Feb 8, 2024

This is related to #19, as well.

@SasaKaranovic
Copy link
Owner

Hi @hawkw
Thanks for the suggestion. This sound like a good idea and it should be relatively easy to implement.
My main concern is making sure that this feature does not break pyinstaller ("distributable") version of the app but we can verify that before merging into master.

@SasaKaranovic SasaKaranovic added the enhancement New feature or request label Feb 8, 2024
@hawkw
Copy link
Author

hawkw commented Feb 8, 2024

My main concern is making sure that this feature does not break pyinstaller ("distributable") version of the app but we can verify that before merging into master.

Yeah, that makes sense, that's mine as well. My assumption is that if the command-line arguments are optional, and if the defaults match the current behavior, then the pyinstaller executable should work exactly the same as it does today --- is that correct?

I'm happy to open a PR making this change, if you like! I don't have a ton of Python experience, but it seems pretty straightforward to add this.

@SasaKaranovic
Copy link
Owner

I would like to as much as possible avoid splitting the code for different platforms.
I glanced over the PR and it's adding features that are used on Linux but would potentially cause issues on Windows and/or pyinstaller which rearranges paths.
Also splitting single application into multiple folders could potentially cause permissions issues.

Would using symlinks or something similar on Linux machine be enough to solve your issue?

@hawkw
Copy link
Author

hawkw commented Feb 9, 2024

I would like to as much as possible avoid splitting the code for different platforms.
I glanced over the PR and it's adding features that are used on Linux but would potentially cause issues on Windows and/or pyinstaller which rearranges paths.

To clarify, the only code in my branch that behaves differently based on the OS is the code for determining the default log file path, and that code was already there; I just moved it to a function so that it could be used to print the default value in the CLI help text.

The only change in behavior in my PR is that the default paths can be overridden through the command-line arguments. If the user doesn't override the default paths, the application will continue its current behavior of storing everything in the same directory as server.py (except for the log file, which was already OS-specific).

Symlinking doesn't really solve the problem because the server will still create files in whatever directory it's installed into. So, if I want to put the server in /usr/bin/, I have to symlink the database and config file into /usr/bin/ as well. That doesn't really achieve the goal of not having runtime state in /usr/bin/.

@SasaKaranovic
Copy link
Owner

I've used software packages that have a "dedicated" install directory and then symlink the executable to /usr/bin. So it seems it's a common practice/solution.

I'm running the VU-Server as a systemd service on Linux, and I'd like to be able to put the VU-Server code in /usr/bin/.

You don't have to install the VU server into /usr/bin, I would actually very strongly advise against that.
However you could install it in your user folder or some common folder and symlink the executable in /usr/bin.
Also running VU server as systemd service does not require that the binary is in /usr/bin directory.

But maybe the better question is, what issue is having all applications files in one directory creating?
For example Apache2 has it's own installation directory and then symlinks bunch of binaries in /usr/bin and/or /usr/sbin.

My main concern is that this change would require additional work, testing and more importantly, potentially create issues later down the line. But offers little to no improvement to the VU Server app. But I'm happy to be proven wrong.

@lf-
Copy link

lf- commented Feb 10, 2024

In any instance where the software is packaged and installed system wide, it is in a directory only writable by the administrator of the system. On Windows you can't write into Program Files as a regular user.

Moving the data that changes to not be in the same place as the code (which you would sincerely hope does not change in the course of an application run) is mandatory in all system-wide installations with a package manager on all platforms.

Command line arguments are always a good idea for doing this; another second idea (which doesn't invalidate the need for command line arguments) is a build-time setting for the defaults for where data is stored, to handle putting it in the appropriate location for wherever the software is packaged.

@hawkw
Copy link
Author

hawkw commented Feb 10, 2024

I'll also add that the documentation currently states that the database is created at a path that is not relative to the location of server.py (see https://docs.vudials.com/server_config/#dial-and-api-configuration):

The SQLite database is store inside user directory

  • Windows: %USERPROFILE%\KaranovicResearch\vudials\vudials.db
  • Linux/Mac: ~/KaranovicResearch/vudials/vudials.db

If this were actually the case, that would also be better than storing the database in "whatever directory server.py is in".

@SasaKaranovic
Copy link
Owner

On Windows you can't write into Program Files as a regular user.

This is not an issue for VU server since it should not run as the regular user.

Maybe I need some additional information to better understand this but right now I'm failing to see what issue would this feature solve or what issue it's causing?

From what I can understand;
VU Server can be installed anywhere.
Systemd/daemon can run an app from anywhere.
Symlinking is an elegant option to add just the binary/executable to /usr/bin folder.

@lf-
Copy link

lf- commented Feb 11, 2024

I work on a Linux distribution. When packaging software for any distribution, packages will put the application files in, say, /usr/lib/vu-server or something like that. This is a directory that is owned by the system package manager and is only writable by root. It can be deleted at any time without concern when the package is removed. This is equivalent to Program Files/vu-server on Windows. We might put a symbolic link in /usr/bin, as well.

The user data, say the database, configuration, and so on, lives somewhere else that cannot be deleted arbitrarily by the package manager, and is writable by the user running the service (which is not root/SYSTEM if we can avoid it). Perhaps on our distro we put that user data in /var/lib/vu-server and give write access to the vu-server user.

The exact location of both where the program code will be and where the user data will be varies by distribution (and isn't your responsibility as a software author to know) but they will almost always be two separate places.

There's several reasons we do this including security: compromising the server does not ensure persistence because it cannot modify its own code. But also, it keeps the data that the user has to back up and actually care about away from the code that is our responsibility as a distribution maintainer and which the user doesn't need to back up as it's in a directory they're not supposed to edit (so nothing of importance in there for backups).

Precisely the same thing would apply with a Windows installer using the standard file paths on the platform: Program Files only contains code that can be deleted on updates, the actual data might go in ProgramData, or the user profile directory someplace. This allows the application to be uninstalled and reinstalled without deleting the user data, and it allows (if the data is in the user profile) multiple users to use the same application with different personal data.

This is why as packaging people we want the runtime data of services separate from the code, because the code should be something we can just wipe away and reinstall at any time.

@SasaKaranovic
Copy link
Owner

I think we are cross talking here.

The original suggestion was Allow overriding config file, database, and other paths from the command line. My question is why is this necessary. More precisely why is specifying an arbitrary location at run time a desired feature instead of, for example just moving the config and db files to a fixed location outside of installation folder?

Please keep in mind that VU Server is managing actual, physical devices and it should not be a user-specific application, it should be a service. What I mean by that is that you can have a VU Server and X number of dials connected to the PC. There will be many processes that are communicating with the server and/or multiple users. The VU Server must always behave consistently for all of them.

The VU Server uses database, config file and www/upload folder to store information about the dials.
For example which background it has, what is the needle/color fading speed, what is the dial name, which API keys have access to which dial etc.
It would be a very bad experience if all of these suddenly became a 'user specific' values, where for one user the server is showing names, configuration, API keys and everything else correctly and then when you run the VU Server as a different user, all of this information is blank/incorrect. Or even worse, third party applications that were configured to work with VU Dials stop working because all previously configured API keys "disappear" because database path has changed arbitrarily.

Again, I agree with the suggestion to separate application data from config data into fixed location.
But I don't see a benefit of allowing arbitrary location switching at run-time and I want to make sure I'm not missing anything before I make a decision and put this into backlog.

@hawkw
Copy link
Author

hawkw commented Feb 12, 2024

The original suggestion was Allow overriding config file, database, and other paths from the command line. My question is why is this necessary. More precisely why is specifying an arbitrary location at run time a desired feature instead of, for example just moving the config and db files to a fixed location outside of installation folder?

Ideally, I think an application should by default respect the locations for state and configuration files provided by the host OS. On most Linux distros, this means that when run as a user process, the application should follow the XDG Base Directory Specification, which in the case of VU-Server would mean storing the database and file uploads in $XDG_DATA_HOME, and searching for the configuration file in the directories given in $XDG_CONFIG_HOME and $XDG_CONFIG_DIRS, storing the lockfile and logs in $XDG_RUNTIME_DIR However, if the application is run as a systemd service, it would be more correct to instead use the systemd-provided environment variables, $RUNTIME_DIRECTORY, $STATE_DIRECTORY, $LOGS_DIRECTORY, and $CONFIGURATION_DIRECTORY, instead. But, it gets more complex: not all Linux distributions use systemd to manage services. The application might be run as a service using another init system, such as runit, in which case those environment variables may not be available. And this is just on Linux; Windows and macOS have their own conventions for where applications should store data.

Implementing all of this correctly requires a fairly complex web of platform-specific code. My sense was that you wanted to minimize the amount of platform-specific code in VU-Server. Furthermore, my feeling was that it was probably better to leave the default behavior as it is, since (as far as I can tell) VU-Server's externally visible behavior doesn't really appear to be versioned, so changing what happens by default could surprise a lot of of existing users. Therefore, I thought it would be preferable to allow the user to opt in by overriding the default locations via the command line, rather than changing the defaults.

Furthermore, making paths configurable on the command line avoids adding the complex code necessary to determine what directories should be used on a particular host OS in VU-Server. On Linux, applications are typically packaged for a particular Linux distribution by that distribution's maintainers. Those distribution maintainers might do things like provide shell scripts for executing an application with the correct paths on that particular distro, and other distro-specific edge cases and conventions. By providing a mechanism to configure the paths used by the server, the task of ensuring that the correct paths on a given distro are used is left up to the distro packagers, rather than VU-Server's maintainers --- which seems like the right thing, because distro maintainers are probably more familiar with their particular system's conventions and oddities.

However, I'm not particularly attached to passing these paths as CLI args. If you are willing to accept a larger amount of platform specific code, I'm happy to instead open a PR that changes VU-Server to respect the XDG base directories and systemd service environment variables when running on Linux. Would you be more willing to accept that change, instead?

@SasaKaranovic
Copy link
Owner

As I mentioned we want the VU Server application to be as platform agnostic as possible. Also it should behave consistently for all users and "share" dial database, configuration and logs across entire system. Making database, configuration and logs user-specific would break that requirement.

Same VU Server installation must behave the same way for all users on the same system.
Regardless who is running the application and from where/how.

Just as a trivial example for the scenario where database/config is user specific or each user specifies where they want their config to be stored;
User A has renamed a dial to "CPU Usage" and then configures the (shared) app X to display CPU usage on this dial.
Then when user B logs in, and starts app X, it could/will fail to work because it will not have the same configuration as the user A and therefore won't have the dial named "CPU Usage", causing an error in the app X.

The other potential problem is that this can cause issues where users "lose" their database, configuration or can't find log files by specifying wrong/different path or running the app as different user. This makes debugging and finding issues much harder, especially with non-technical users.

As far as I can tell this is not an issue or a blocker but more of a "nice to have" feature specifically targeting Linux users.
Right now this would require non-negligible amount of work to make sure this feature is implemented and tested to work properly on different Linux distros and also make sure it doesn't cause issues on other platforms like Windows and Mac OS.
Also it would require a solution to make sure that dial configuration, database (and potentially logs) behave the same way for all users on the same system.

With that said, I will add a to-do item for future development to consider having more flexibility when it comes to specifying database, config and log paths.

Last but not least, I hope you (and everyone else) understands that I appreciate all of this feedback and would love to be at the point where the VU Server behaves nicely on all platforms while also following all the different philosophies and specs that come with using Windows, Linux and Mac OS platforms.
However if the change is purely motivated by adhering to certain philosophy or specification with little to no benefit to anything else, or worse, potentially opening avenues for issues and confusion. Then it does not mean that it will never happen, but it will not be prioritized over other features that offer more tangible benefit.

@hawkw
Copy link
Author

hawkw commented Feb 13, 2024

As I mentioned we want the VU Server application to be as platform agnostic as possible. Also it should behave consistently for all users and "share" dial database, configuration and logs across entire system. Making database, configuration and logs user-specific would break that requirement.

Same VU Server installation must behave the same way for all users on the same system. Regardless who is running the application and from where/how.

Using the OS' conventional state and configuration directory paths is in service of both of these goals. Since VU-Server is intended to be run as a service that's always running in the background, for all users, running it as a systemd service unit is the ideal way to run VU-Server on Linux. In order to be a well-behaved systemd service, a program should respect the directories that systemd provides the service for storing its configuration and state. For example, when running as a systemd service, systemd might set the $STATE_DIRECTORY environment variable to /var/lib/vu-server. Using this directory for the VU-Server database file ensures that the database is stored in a location where it is persisted across reboots, and that a single global instance of the database exists for the system. But, the specific directory that systemd is configured to set as the state directory for a service may vary based on distribution, so it's necessary to use the environment variable rather than hard-coding /var/lib/vu-server.

As far as I can tell this is not an issue or a blocker but more of a "nice to have" feature specifically targeting Linux users.

I disagree with this to some extent. It isn't an issue that makes VU-Server unable to run on Linux at all, but it is a blocker for packaging VU-Server for most Linux distros, and it makes running VU-Server as a systemd service difficult on many Linux distributions, such as those where the /bin filesystem is immutable, or those where the runtime directory for systemd services is a temporary directory that's deleted on reboots. While it's certainly possible to execute VU-Server on Linux, attempting to run it as a service may result in the database being reset every time the system reboots (when systemd services are run in a temporary directory), or the program crashing on startup (on systems where the directory the server is installed to is not writable).

I don't care what specific solution (configuration via the CLI, or honoring environment variables defined by the host OS) is implemented, but I do think this is a problem for Linux users that makes using the application quite painful.

Last but not least, I hope you (and everyone else) understands that I appreciate all of this feedback and would love to be at the point where the VU Server behaves nicely on all platforms while also following all the different philosophies and specs that come with using Windows, Linux and Mac OS platforms.

Thanks for taking the time to say so, and I appreciate that you've taken the time to read and participate in this discussion, even though we haven't agreed about the right solution. I'll also add that I've had a ton of fun playing with my dials, and I'm working on writing my own software for displaying metrics on them --- which I'm looking forwards to sharing soon! Thanks for all your hard work on this project!

@hawkw
Copy link
Author

hawkw commented Feb 13, 2024

I disagree with this to some extent. It isn't an issue that makes VU-Server unable to run on Linux at all, but it is a blocker for packaging VU-Server for most Linux distros, and it makes running VU-Server as a systemd service difficult on many Linux distributions, such as those where the /bin filesystem is immutable, or those where the runtime directory for systemd services is a temporary directory that's deleted on reboots. While it's certainly possible to execute VU-Server on Linux, attempting to run it as a service may result in the database being reset every time the system reboots (when systemd services are run in a temporary directory), or the program crashing on startup (on systems where the directory the server is installed to is not writable).

As a concrete example, I can't run VU-Server as a systemd service with ProtectHome=yes or DynamicUser=yes, which are the recommended settings for long-running services that are exposed to the network, for security reasons. The server crashes on startup when run as these service types, because it always attempts to write log files to /home/{user}/vudials.log, and the home directory for these services types is not writable.

@SasaKaranovic
Copy link
Owner

Ok, I think that sooner or later this change will be in.
So maybe let's change focus to how we want this implemented (whenever time comes for doing so).
Keep in mind that we want to keep this platform agnostic, so let's not focus just on Linux/Mac or just on Windows.

From my end, I will make the change to have all currently hard-coded paths moved to a single file/class that rest of the app uses.
This would hopefully clean up and simplify how we get/set those paths.
There also needs to be an "upgrade" mechanism so existing users don't lose their db, config etc or have to do anything when this change becomes active.

I am still concerned that having paths as command line arguments would cause issues where for whatever reason these paths "become" user-specific, there is a typo and then everything breaks. I would like something more "permanent" but not hard-coded.

I would also like to keep all the VU server data (config, database, log and upload folder) on the same shared path.
For example have folder .../KaranovicResearch/VUServer/ and inside have config.yaml, vudials.db, server.log and subfolder for www/upload. This will make things easier when it comes to permissions, debugging, finding/backing-up files etc.

One easy way to set this custom path across multiple systems (and also make it not so easy to run the app with wrong parameters) would be to have a system environment variable, ie. VUSERVER_DATA_LOCATION that would contain the path where data should be stored.
If this environment variable does not exist, then the default version would be used (ie. Win: C:\Users\{USER}\KaranovicResearch\VUServer\ and on Linux/Mac /home/{user}/KaranovicResearch/VUServer/
And maybe have a feature where if this environment variable is changed, all files from the previous location would be moved to the new location.

Let me know what you guys think or if you see any issues.

I'm working on writing my own software for displaying metrics on them --- which I'm looking forwards to sharing soon!

That's great! We are going to add a list of community made applications, plugins and software that support VU Dials.
So people can easily find apps that they can use with their VU1s.
Once you are happy with how they work, we can add them to the list.

@hawkw
Copy link
Author

hawkw commented Feb 15, 2024

One easy way to set this custom path across multiple systems (and also make it not so easy to run the app with wrong parameters) would be to have a system environment variable, ie. VUSERVER_DATA_LOCATION that would contain the path where data should be stored.
If this environment variable does not exist, then the default version would be used (ie. Win: C:\Users\{USER}\KaranovicResearch\VUServer\ and on Linux/Mac /home/{user}/KaranovicResearch/VUServer/
And maybe have a feature where if this environment variable is changed, all files from the previous location would be moved to the new location.

This approach sounds good to me! As long as it's possible to configure VU-Server to store data at a path outside of the installation directory, platform-specific conventions like storing logs in /var/log can be accomplished by downstream code using symlinking. And, a single environment variable is definitely much less complexity in the server itself, which seems good.

@SasaKaranovic
Copy link
Owner

There is a PR #29 that should implement all of the changes we discussed.
Please give it a try when you get a chance and let me know if you encounter any issues (in the PR comment or here).

@SasaKaranovic
Copy link
Owner

@hawkw @lf- Did you had a chance to test this PR and see if it solves this issue?

@hawkw
Copy link
Author

hawkw commented Mar 1, 2024

I haven't yet, but I'll try to give it a look today or this weekend. Thank you!

Copy link

github-actions bot commented May 6, 2024

This issue is stale because it has been open 15 days with no activity. Remove stale label or comment or this will be closed in 3 days.

@github-actions github-actions bot added the Stale label May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants