Transport & Engine: factor out getcwd() & chdir() for compatibility with upcoming async transport

[khsrali]

Transport.getcwd() & Transport.chdir() methods used to making life easier by allowing basically all Transport methods to function with relative paths.

However, for upcoming async changes, this was problematic, as several calculations ended up writing and fetching from wrong directories.

This PR, gets ride of the habit of setting working directory for an entire instance of Transport class in engine.
Only because in aiida-core several calculations use a same instance of the Transport, (to avoid opening many channels, etc.)

[codecov]

Codecov Report

Attention: Patch coverage is 85.45455% with 16 lines in your changes missing coverage. Please review.

Project coverage is 77.90%. Comparing base (ef60b66) to head (b3ca799).
Report is 132 commits behind head on main.

Files with missing lines	Patch %	Lines
src/aiida/engine/daemon/execmanager.py	85.46%	8 Missing ⚠️
src/aiida/calculations/monitors/base.py	20.00%	4 Missing ⚠️
src/aiida/orm/nodes/data/remote/base.py	80.00%	2 Missing ⚠️
src/aiida/transports/transport.py	87.50%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6594      +/-   ##
==========================================
+ Coverage   77.51%   77.90%   +0.40%     
==========================================
  Files         560      567       +7     
  Lines       41444    42147     +703     
==========================================
+ Hits        32120    32830     +710     
+ Misses       9324     9317       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[GeigerJ2]

Thanks, @khsrali! I think it's definitely an improvement not to rely on getcwd and chdir anymore. Quite a lot of changes to unpack. Given that all tests are passing, I assume it works, so I'd approve. Though, maybe we can discuss the StrPath class again in person, I don't think I fully understand its use.

[GeigerJ2]

Thanks, @khsrali! I think it's definitely an improvement not to rely on getcwd and chdir anymore. Quite a lot of changes to unpack. Given that all tests are passing, I assume it works, so I'd approve. Though, maybe we can discuss the StrPath class again in person, I don't think I fully understand its use.

[unkcpz]

Given that all tests are passing, I assume it works, so I'd approve. Though, maybe we can discuss the StrPath class again in person, I don't think I fully understand its use.

I did a quick look on the PR, I have the same question on this. Would be nice to just use Pathlib I think.

[khsrali]

Thanks a lot @GeigerJ2 for the review. Please check the new changes.

Though, maybe we can discuss the StrPath class again in person, I don't think I fully understand its use.

Yes! as we discussed, I mainly suggested that for aesthetic reasons, I didn't want to call for str() every time passing a path to any of transport methods as they only accept string, for now.

Anyways, now I changed to pathlib, at some point we need to improve the whole transport interface to accept and work with path-like-objects instead of str, but that is out of scope of this PR.

[khsrali]

Reminder for me:
1) update this doc string of exec_command_wait_bytes:
Enforce the execution to be run from the pwd (as given by self.getcwd), if this is not None.
2) this line in sshp.py:listdir() should come after setting base_dir

python~ ~if not pattern:~ ~ return self.sftp.listdir(path)~ ~

[khsrali]

@GeigerJ2 can you have a final look at this?
kinda need this to merge to avoid conflict hell with the new PR I'm going to make, soon.
Cheers

[GeigerJ2]

Nice, pathlib for the win. Thanks for adapting the code, @khsrali. Just let's fix the fixtures, then I'll approve!

[GeigerJ2]

Nice, pathlib for the win. Thanks for adapting the code, @khsrali. Just let's fix the fixtures, then I'll approve!

[GeigerJ2]

fml lgtm!