fix: symbolic link creation for meltano.exe when on windows

[BuzzCutNorman]

This fixes the bug #6467 where on Windows meltano upgrade stops because MeltanoInvoker.invoke() is calling subprocess.run() which requires a executable target. Since the symbolic link .meltano\run\bin is targeting meltano not meltano.exe a failure occurs. I updated the Project.activate() function to target meltano.exe when Windows is detected.

This does not fix any previously created symbolic links that might exist in projects.

Close #6467

[netlify]

👷 Deploy request for meltano pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	3113106

[codecov]

Codecov Report

Merging #6468 (3113106) into main (c86cca4) will decrease coverage by 0.03%.
The diff coverage is 27.27%.

@@            Coverage Diff             @@
##             main    #6468      +/-   ##
==========================================
- Coverage   88.71%   88.68%   -0.04%     
==========================================
  Files         283      283              
  Lines       20457    20465       +8     
  Branches     2017     2020       +3     
==========================================
  Hits        18149    18149              
- Misses       1973     1979       +6     
- Partials      335      337       +2     

Impacted Files	Coverage Δ
src/meltano/core/project.py	88.11% <27.27%> (-3.12%)	⬇️
src/meltano/core/plugin_location_remove.py	86.73% <0.00%> (-4.09%)	⬇️
src/meltano/cli/utils.py	83.80% <0.00%> (+1.05%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[visch]

Still need to dive into this a bit more, we should enable the upgrade test on Windows as well. https://github.com/meltano/meltano/blob/main/tests/meltano/cli/test_upgrade.py#L12 is probably the right test to be "reenabled for windows here!)

[BuzzCutNorman]

Vish, Would you like me to comment the if statement out save, commit, and publish to see what happens ?

[visch]

Vish, Would you like me to comment the if statement out save, commit, and publish to see what happens ?

Well I'm trying to understand what we're fixing here. meltano upgrade actually runs fine for me locally. So it's just meltano ui that doesn't.

A test helps clear up what code path(s) we're taking here that caused this issue. Maybe it's not easy to do because of the UI?

[BuzzCutNorman]

This is an edge case that I have run into. If you run dir .\.meltano\run inside a project folder and you see the file bin the upgrade will fail. This files is created when a project is activated. If .\.meltano\run\bin is present and you run meltano upgrade the you will get the error if it is not present you won't.

You are correct this is separate from the UI issue you filed. That is why I created a new bug issue. Though it could in some cases be a root cause of a UI upgrade failure.

[BuzzCutNorman]

I guess I don't even know what purpose this bin symbolic link has or if it is even needed anymore. Windows fails out at initialization since it can't find a meltano file and hasn't been creating a bin. I wonder if maybe what needs to be done is instead of pointing it to the meltano.exe. I just need to make sure always fails and never creates a bin file for windows for any enviroment venv, pipx, poetry .

[BuzzCutNorman]

If you want to see what the symbolic link .\.meltano\run\bin is targeting in Windows run this from a powershell prompt while in a Meltano project folder.

dir .\.meltano\run\ -recurse -force | ?{$_.LinkType} | select FullName,LinkType,Target

I think you can do this other ways but I found this one at it has worked well for me.

[visch]

@BuzzCutNorman nice work this works great on Windows (as long as you run as an administrator, but I think tackling that in #6160 makes sense as we'd probably want to offer junctions and symlinks re https://stackoverflow.com/questions/9042542/what-is-the-difference-between-ntfs-junction-points-and-symbolic-links )

[visch]

@edgarrmondragon looks good to me! Unless we can get rid of .meltano\run\bin which sounds like a no go as it's needed for this and airflow (maybe others?)

[BuzzCutNorman]

@visch that is wonderful to hear this PR is working great for you as well. I know it stinks Windows requires administrator rights to create symlinks. I looked into the ntfs junctions. Junction can only point to directories. (hard links are the same style but point to files). Since the code is expecting the destination to be a file a junction would cause an error. The hard links which I think you create with os.link() point to files and might work.

• https://docs.microsoft.com/en-us/windows/win32/fileio/hard-links-and-junctions
• https://docs.microsoft.com/en-us/windows/win32/fileio/creating-symbolic-links
• https://docs.python.org/3/library/os.html#os.link
• https://docs.python.org/3/library/os.html#os.symlink

[BuzzCutNorman]

@visch just tried to use a hard link (os.link()) and it works if you are an administrator 😄 but it doesn't if you are just a user. 😿

[BuzzCutNorman]

I might have been wrong. 😸 In the poetry virtual environment for the user account there is no meltano.exe present. I am going to try and have it create a link to the meltano.cmd file that is present. If a hard link is created, then yay we are a little closer to not needing to run as an administrator.

[BuzzCutNorman]

@visch Yay!!! it did create a link to the meltano.cmd file 🥳 . I committed the change and just watched the automated GitHub tests fail for Windows since they are setup to create hard links across drives. That is a limitation of hard links, you have to stay on the same drive. 😞 . I can see people keeping python installed on one drive and the data projects on another so I don't think hard links are going to be our answer.

[BuzzCutNorman]

@visch all the tests pass now. This version uses the symlink_to() but I put in place a test to see if the user is an administrator. Please test this version out when you get a chance.

[WillDaSilva]

Save for a few style changes and typo corrections, this looks good to go. Thanks @BuzzCutNorman!

[BuzzCutNorman]

@WillDaSilva Thank You, for clean up my code. Looks much better now. I was testing it out and noticed that when I use --log-level debug I am not seeing the messages about the file not being found or that you don't have admin privileges. I used logger.debug is there a different logger I should be using now?

[WillDaSilva]

I was testing it out and noticed that when I use --log-level debug I am not seeing the messages about the file not being found or that you don't have admin privileges. I used logger.debug is there a different logger I should be using now?

@BuzzCutNorman That's strange - I don't know why that would be. I'll see if I can reproduce this later today.

[WillDaSilva]

@BuzzCutNorman I have reproduced the problem you mentioned, where logger.debug does not result in any output after being called, despite the log level being set to debug. Nice catch by the way. It does emit a log line if logger.info or a higher-level log function is called instead.

After some digging, it seems like the issue is that the the log config is only applied after the project has been activated, and so when logger.debug is called it will always be using the default log config. This seems tricky to work around, so I propose for this PR we simply promote the logger.debug calls to logger.warn, since that seems appropriate for these two messages imo.

This is probably an issue for other logger.debug calls that are made before the log config is applied, so I'll open an issue to address that in another PR.

[BuzzCutNorman]

@WillDaSilva Thank you for taking the time to solve that mystery. I changed the messages to use logger.warn as suggested.