[Bug]: 'Open File' dialog when editing configuration files in Environment properties prepends CWD of client, doesn't resolve variables

[kranskydog]

Apache Hop version?

2.6.0

Java version?

openjdk version "17.0.8" 2023-07-18 LTS OpenJDK Runtime Environment (Red_Hat-17.0.8.0.7-2.0.1) (build 17.0.8+7-LTS) OpenJDK 64-Bit Server VM (Red_Hat-17.0.8.0.7-2.0.1) (build 17.0.8+7-LTS, mixed mode, sharing)

Operating system

Linux

What happened?

This occurs in both Hop-GUI and Hop-Web clients

1. Edit an Environment
2. click 'New' to create a new file, or, if you have existing files which have and environment variable in the path (ie ${HOP_CONFIG_FOLDER}) choose that file and click 'Select
3. the 'Open File' dialog will open with the path of the CWD prepended to the path of the selected configuration file, and/or without any environment variables being replaced

Issue Priority

Priority: 3

Issue Component

Component: Hop Gui, Component: Hop Web

[mattcasters]

So I guess this is a feature request to allow variables to be replaced in the Open File dialog?
Should we retain the value somehow or just replace the expression with its value?

[kranskydog]

It is trying to look for a file. How is it going to find the file if it is looking under the literal variable name, instead of the value of the variable?

similarly, when it does substitute the value of the variable, when that value is an absolute path, prepending the CWD of the client isn't going to help

[sramazzina]

@hansva looking at the last reference I think we can close this issue, isn't it?

[kranskydog]

@sramazzina @hansva That reference is wrong
@mattcasters there is a typo in the description of your pull request #3324 -> #3234

[hansva]

yup, this can stay open.

[hansva]

Actually, I just tried this and variable replacement works.
When defining a variable in hop-config.json it can be used in the environments dialog.
Where did you specify your HOP_CONFIG_FOLDER?

[kranskydog]

As far as I am aware, there is only one place it needs to be set - the user's environment before they launch the GUI, or the environment of the application server for Hop Web
https://hop.apache.org/manual/latest/installation-configuration.html#_the_environment_variables_to_set

[hansva]

That expansion also works on my system and it uses the correct variable value...

hans@MacBook-Pro-2:~|⇒  echo $HOP_CONFIG_FOLDER
/Users/hans/config

[kranskydog]

@hansva and what happens when you hit 'Select' on that file?

[hansva]

No issue there for me.
Do I see it correctly that your hop_config_directory points to a smb/windows share //u01 ?
Because that might be the cause of all issues, we have known issues with UNC paths.

[kranskydog]

@hansva No, it's all on Linux

[kranskydog]

somehow it seems to be remembering where it was last time, because when I hit 'Select' I got

which the last path I used to save a pipeline
But then, while I am here

if I hit 'New', I get

and then if I hit 'Select' again, I get

So, simply doing it once isn't necessarily going to reproduce the problem

[kranskydog]

for reference
[apache@apchop01 ~]$ pwd
/home/apache
[apache@apchop01 ~]$ echo $HOP_CONFIG_FOLDER
/u01/app/hop/admin/config
[apache@apchop01 ~]$ /u01/app/hop/product/hop-client-2.6.0/hop-gui.sh

[kranskydog]

Note, also
"projectName" : "oracle",
"projectHome" : "${HOP_CONFIG_FOLDER}/projects/oracle",
"configFilename" : "project-config.json"
} ],
"lifecycleEnvironments" : [ {
"name" : "XE",
"purpose" : "Development",
"projectName" : "oracle",
"configurationFiles" : [ "${PROJECT_HOME}/environments/XE-config.json", "${PROJECT_HOME}/environments/XE-passwd.json" ]
}, {
"name" : "ORCL",
"purpose" : "Production",
"projectName" : "oracle",
"configurationFiles" : [ "${PROJECT_HOME}/environments/ORCL-config.json", "${PROJECT_HOME}/environments/COMMON-config.json" ]

[hansva]

Aha, I was able to reproduce now.

I was missing 1 small part of your puzzle. which seems to be the root cause of all your problems.

Project home does not support variables. Or at least on expansion, it is unable to replace the nested parameter
${PROJECT_HOME} expands to ${HOP_CONFIG_FOLDER}/projects/oracle and then the ${HOP_CONFIG_FOLDER} is not replaced by the value.

When pressing the new button it tries to go to ${PROJECT_HOME} it can't find the file location and we have a fallback in place to go to PWD if locations are not found and this ends up in your weird path.

In the file dialog when pressing the "P" button in the top left button it should go to your project root, but this also fails.

As for your comment on Select, yes the dialog remembers the last known location and goes there by default. The starting point used to be project root always. but we had complaints from users that were working in a folder that they always had to browse there. So now we remember the last location and use that as starting point. We have buttons in the top left to go to user home or project root.

[kranskydog]

"Project home does not support variables."

so, if I move my HOP_CONFIG_FOLDER, which includes all my project homes, instead of just changing the value of HOP_CONFIG_FOLDER, and having it Just Work(TM), I also have to go through and change all the values of PROJECT_HOME, because it isn't smart enough to resolve the HOP_CONFIG_FOLDER variable.

[mattcasters]

It's just a strange design you made, one we didn't encounter yet. That's all really, Phil. Most people would keep configuration and project metadata separate in the light of putting both in separate version control repositories.
We didn't say we wouldn't fix it.

[kranskydog]

Ahem...

[apache@apchop01 config]$ pwd
/u01/app/hop/product/hop-client-2.6.0/config
[apache@apchop01 config]$ ls
hop-config.json projects
[apache@apchop01 config]$ ls projects
default samples

[apache@apchop01 config]$ pwd
/u01/app/hop/admin/config
[apache@apchop01 config]$ ls
hop-config.json metadata projects
[apache@apchop01 config]$ ls projects/
default oracle samples

I am literally following the example provided in the standard distribution.

Not to mention (from https://hop.apache.org/dev-manual/latest/hopweb/index.html#_modify_startup_script)

[mattcasters]

Just to clarify further Phil, we absolutely do not recommend that you use the default configuration. But, I guess it's great that we now know how you got to your setup.

[kranskydog]

so what you're telling me is HOP_CONFIG_FOLDER should only contain one file.

[mattcasters]

Thanks for helping us with the reproduction of the issue Phil. It's been informative.

[kranskydog]

Cf

[hansva]

It's valid usage and it should work, so we will fix it.

[kranskydog]

Just for some background, I am not a developer, I am an infrastructure person (I manage a large Oracle database installation). So my POV comes from what is the architecture, how would I deploy this, and what are the infrastructure lifecycle management implications. I am not so concerned with things like managing the development lifecycle - that's S.E.P. One big thing I try to use is 'single source of truth', and the corollary that things should only have one place where they are defined, and then that definition can be used by reference. If it changes in the architecture, it should only have to be changed once in the configuration, and everything else that references it Just Works(TM)

So I am not trying to be unnecessarily antagonistic, I am just passionate about well designed and managed infrastructure. I am hopeful that one day HOP may have a place in that where I work.

The other thing is that while I am technically adept at many things, I come to this as a novice, so will take what is in the documentation and the provided examples at face value.

[mattcasters]

Thanks again for the background information Phil. As a project Apache Hop doesn't really want to dictate a preferred way to set up, use or install. At that same time, the people that built it, decided on some things in the past. Making Apache Hop doing a nested variable resolution here will surely help more than just you and open up more possibilities.
I would personally limit the solution to this particular HOP_CONFIG_FOLDER resolution. I'll create another issue to see how we can make nested variable resolution more generic and configurable across the platform.

[mattcasters]

Created issue #3454