`.do(onSubscribed:)` and `.do(onSubscribe:)` don't respect `.observe(on:)`

[c128128]

Short description of the issue:

1 Case

Looks like .do(onSubscribed:) and .do(onSubscribe:) don't respect .observe(on:).

example:

  Observable<String>.just("1")
      .observe(on: SerialDispatchQueueScheduler(internalSerialQueueName: "observe"))
      .do(onSubscribed: {
          print("do, isMain ==> \(Thread.isMainThread), DispatchQueue ==> \(DispatchQueue.name())")
      })
      .subscribe(onNext: { _ in
          print("subscribe, isMain ==> \(Thread.isMainThread), DispatchQueue ==> \(DispatchQueue.name())")
      })

result:

do, isMain ==> true, DispatchQueue ==> main-thread
subscribe, isMain ==> false, DispatchQueue ==> observe

expected:

do, isMain ==> false, DispatchQueue ==> observe
subscribe, isMain ==> false, DispatchQueue ==> observe

2 Case

Also might be related:

Looks like .do(onSubscribed:) and .do(onSubscribe:) don't respect .subscribe(on:) if it is above do statement.

example:

Observable<String>.just("1")
    .subscribe(on: SerialDispatchQueueScheduler(internalSerialQueueName: "subscribe"))
    .do(onSubscribed: {
        print("do, isMain ==> \(Thread.isMainThread), DispatchQueue ==> \(DispatchQueue.name())")
    })
    .subscribe(onNext: { _ in
        print("subscribe, isMain ==> \(Thread.isMainThread), DispatchQueue ==> \(DispatchQueue.name())")
    })

result:

subscribe, isMain ==> false, DispatchQueue ==> subscribe
do, isMain ==> true, DispatchQueue ==> main-thread

expected:

subscribe, isMain ==> false, DispatchQueue ==> subscribe
do, isMain ==> false, DispatchQueue ==> subscribe

[gigi206]

Note that msys2 readlink binary found the good path:

PS C:\VSCode-Anywhere\apps\scoop\apps\7zip> C:\VSCode-Anywhere\apps\msys2\usr\bin\readlink.exe C:\VSCode-Anywhere\apps\scoop\apps\7zip\current
/c/VSCode-Anywhere/apps/scoop/apps/7zip/19.00
C:\VSCode-Anywhere\apps\msys2\usr\bin\cygpath.exe -w /c/VSCode-Anywhere/apps/scoop/apps/7zip/19.00
C:\VSCode-Anywhere\apps\scoop\apps\7zip\19.00

[arizvisa]

Ftr since junctions are actually different from links, It'll require platform-specific code.

Here's a reference for the devers: https://eklausmeier.wordpress.com/2015/10/27/working-with-windows-junctions-in-python/

[gigi206]

I think it must be include in file.readlink.

I have created a custom module for this case based on your link. I hope it will be integrated in the future.

[arizvisa]

If you made a PR, can you reference it in this issue? That way us users can merge it into our own salt/modules/file.py before the devers get around to merging. The maintainers have go through a potentially long process and require testcases before merging bugfixes like this.

[xeacott]

I looked through the files related to this, and it appears we don't currently support the usage of junctions. I believe it can be handled in the win_file.py module but would be more appropriate in utilts/path.py. We have a few functions in here such as def readlink(path) and def islink(path) which re-implement os.readlink() and os.islink() but don't seem to handle Window's junctions.

That would be awesome if you could submit a PR @gigi206 😄

[gigi206]

I have integrated the readlink quick and dirty in this module https://github.com/gigi206/VSCode-Anywhere/blob/salt/_modules/scoop.py#L82 (I rewrite the code of my tool VSCode-Anywhere with Saltstack (from powershell and shell), code is in active development in this branch, alpha version)

[arizvisa]

Actually while thinking about this, maybe I can incorporate it into my PR. I submitted PR #55000 which adds hard links to posixy environments. Since you can't hard link a directory in posix, I simply return a failure.

However, ntfs junctions are pretty similar to hard links (yet for directories). Do you guys think it'll make sense to add windows support and merge this functionality into that PR?

[arizvisa]

(This would have the effect of file.hardlink being usable with directories on Windows, but not on Posix.)

The one issue is that with hard links you can't read its target, yet you can with junctions.

[xeacott]

Well I'm thinking that can be solved by just putting junction specific code in win_file.py. That will just be scooped up by Salt if we're on a windows system, but both win_file.py and file.py execution modules both call functions in utils/path.py file.

[arizvisa]

Ok. I'll leave it alone as a completely separate implementation then..

[xeacott]

Actually while thinking about this, maybe I can incorporate it into my PR. I submitted PR #55000 which adds hard links to posixy environments. Since you can't hard link a directory in posix, I simply return a failure.

However, ntfs junctions are pretty similar to hard links (yet for directories). Do you guys think it'll make sense to add windows support and merge this functionality into that PR?

The reason I would like to see it separate is because of our CI/CD system. Having separate PR's with platform specific code would make potential debugging easier and gives us more granularity in the future.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[stale[bot]]

Thank you for updating this issue. It is no longer marked as stale.

[sagetherage]

@xeacott is this a bug or a feature - they are mutually exclusive. Pick one.

[arizvisa]

ftr, op's implementation is at https://github.com/gigi206/VSCode-Anywhere/blob/2a55aec128ee9234b7fa6d3f9610e4a5878d1403/_modules/scoop.py#L81, and it's based on the implementation at https://eklausmeier.wordpress.com/2015/10/27/working-with-windows-junctions-in-python/.

[xeacott]

This is mostly a feature as it will extend support. Still in favor of this 👍

[twangboy]

This should be fixed with #68066. This was part of the most recent release (3006.13/3007.5)