Optimization of Config File Path Handling with Environment Variable S…

[lcolok]

Hello! I've noticed that the current project has some limitations in managing the paths of configuration files, especially across different environments. To enhance the flexibility and maintainability of the project, I propose a small but significant improvement as follows:

I've introduced a new WAS_CONFIGS_DIR parameter, designated for storing various .json configuration files. WAS_CONFIGS_DIR first attempts to retrieve its value from the system environment variable WAS_CONFIGS_DIR. If the environment variable is not set, it defaults to using WAS_SUITE_ROOT as the directory for configuration files. The advantage of this change is that it offers users greater flexibility to define the location of their configuration files as per their requirements. Moreover, this modification is backward compatible—if users have not set any environment variable, the project will continue to use WAS_SUITE_ROOT as the storage path for configuration files as before. This means that this change will not affect the normal usage of existing users.

I kindly ask you to consider accepting this modification. I believe that it will make the project more flexible and powerful in its future development. Should there be any questions or further discussions needed, please feel free to contact me.

Thank you once again for your hard work and dedication to this project!

[WASasquatch]

Thanks for your evaluation, and consideration of multi-env systems. This seems like an appropriate change imo, and I will merge it.

[Arcitec]

Good idea! It's a bit weird to call it WAS_CONFIGS_DIR when it's actually a WAS_CONFIG_DIR though. There's only one config in that dir. Not plural.

[lcolok]

Good idea! It's a bit weird to call it WAS_CONFIGS_DIR when it's actually a WAS_CONFIG_DIR though. There's only one config in that dir. Not plural.

Thank you all for your engagement and insightful feedback regarding this recent submission, particularly addressing Arcitec's comments. I'd like to delve deeper into some crucial aspects:

1. Anticipating Future Configuration Needs: While our current project setup involves just a single configuration file, the evolving nature of our project, especially with the integration of custom nodes, could necessitate splitting configurations into multiple files. Adopting the plural WAS_CONFIGS_DIR provides the necessary foresight for potential future expansions.

2. Containerization Strategy: My predominant deployment strategy involves Docker-based containerization. In this context, mounting directories from the host into the container has proven to be a more common and robust approach compared to single file mounts. This directory-based approach significantly enhances our flexibility in managing read/write permissions and overall system stability.

3. Configuration File Management within Containers: Consider the scenario of initializing a custom node for the first time, which requires generating a default configuration file. This process is straightforward with directory mounts, as it allows for dynamic creation and modification of configuration files within the container. Conversely, single file mounts necessitate pre-configuration on the host side, a process that complicates initial setup and subsequent modifications.

4. Handling Unanticipated Challenges: Practical deployment often throws up a range of read/write and permission challenges, particularly in a containerized environment. Opting for directory mounts over single file mounts offers a more adaptable and robust solution to these challenges, thereby enhancing operational flexibility and system reliability.

These clarifications highlight the thought process behind proposing WAS_CONFIGS_DIR and its potential benefits as our project evolves. I sincerely appreciate the opportunity to contribute and look forward to the ongoing collaborative efforts to enhance our project.

[Arcitec]

@DigitalIO Hi Michael. I understand your concerns, but that is only the variable for WAS (WAS Node Suite) configuration, which is one configuration per directory. That is why the variable name should be singular.

You can of course have other ComfyUI nodes that also use other configuration directories, and those might work differently. But WAS is "1 dir = 1 config". :) Switching that singular environment variable WAS_CONFIG_DIR to another location is how you can achieve multiple different configs for WAS. But only one config is active at a time, and switching requires changing that variable and restarting ComfyUI.

Oh and thank you for contributing this change. It's very nice to be able to switch the WAS config easily.

[DigitalIO]

I think you have me confused for someone else

[Arcitec]

@DigitalIO Yeah, sorry about that. Typing @ and then "Ico" (the first letters of OP's username) brings up your name instead, and I didn't double check.

The previous message was for @lcolok.

[lcolok]

You pointed out that using the singular form for naming WAS_CONFIG_DIR is more appropriate, and I now agree with this perspective. Indeed, in the current state of the project, the singular form seems more direct and accurately reflects the fact that there is only one configuration file in each directory. However, since the current PR has already been merged, the need to change from a plural to a singular form can be addressed in another PR. Considering your clear understanding and suggestion on this matter, I believe it would be a good idea for you to initiate a new PR to adjust the naming. Such a correction will help improve the accuracy and standardization of our code. Promptly making this change is beneficial. As it stands, the impact of this modification should be within a manageable range and will not affect many users. 😁

[WASasquatch]

I think he already fixed it to singular in a PR after yours: https://github.com/WASasquatch/was-node-suite-comfyui/pull/307/commits/426b3715ae1e88517bf61f8c6365beac7669e9fd

[Arcitec]

I think he already fixed it to singular in a PR after yours.

Correct. And it was merged about an hour after this PR, so all is good. :)