feat: os agnostic file existence check

[jenhagg]

Purpose

Small chunk of work to support windows natively. This one splits the file existence check, (basically an ls command) into one for LocalDataAccess which is os independent, and the one for SSHDataAccess which remains unchanged.

What the code is doing

Split implementation from base class, and some related cleanup.

Testing

Ran through creating and preparing a scenario in a container, so we exercise the base functionality and local implementation. Again, the server use case is not changed.

Time estimate

5 min

[BainanXia]

I presume the reason we put this down here is for situations in the future, we have other classes that inherit DataAccess without overwriting _exists method?

[jenhagg]

Yep, that's pretty much it. I'd say a couple related reasons are 1) make it so that self._exists is defined in the context we're referencing it (make editor diagnostics happy), and 2) it's a form of documentation, so you know what needs to be implemented in a subclass.

[BainanXia]

This is smart.

[BainanXia]

Looks good.

[kasparm]

@jon-hagg I am reviewing some code and checked this if statement. As I understand, I enter the if statement if result == False which returns me the "already exists" error message in case should_exist==False and result== False not sure that is the expected behavior.
Also, why not simplifying this check, I had to bend my head :)

[jenhagg]

Yeah it's a bit head bending.. I figured it's worth it if the usage is clear externally (treating the function as a black box). In your example, if should_exist==False and result==False, then we'd have the compare operator set to == so it becomes if False == True, and we won't enter the if statement.

[jenhagg]

I also tested it just to make sure..

In [7]: from powersimdata.data_access.data_access import LocalDataAccess

In [8]: lda = LocalDataAccess()

In [9]: lda.root
Out[9]: '/Users/jonhagg/ScenarioData/'

In [10]: %ls /Users/jonhagg/ScenarioData/
ExecuteList.csv      ScenarioList.csv     old/                 test1.txt
ExecuteList.csv.bak  data/                raw/                 tmp/

In [11]: lda._check_file_exists('test1234.txt', should_exist=False)

In [12]: lda._check_file_exists('test1.txt', should_exist=True)

In [13]: lda._check_file_exists('test1.txt', should_exist=False)
---------------------------------------------------------------------------
OSError                                   Traceback (most recent call last)
<ipython-input-13-78874ba83f1c> in <module>
----> 1 lda._check_file_exists('test1.txt', should_exist=False)

~/code/PowerSimData/powersimdata/data_access/data_access.py in _check_file_exists(self, path, should_exist)
    148         if compare(result, True):
    149             msg = "not found" if should_exist else "already exists"
--> 150             raise OSError(f"{path} {msg} on {self.description}")
    151
    152     def makedir(self, path):

OSError: test1.txt already exists on local machine

[kasparm]

You are right. I did compare(result, should_exist) instead of compare(result, True) in my head.
Why not using:

        exist = self.fs.exists(path)
        if should_exist and not exist:
            raise OSError(f"{path} not found")
        if exist and not should_exist:
            raise OSError(f"{path} already exists")

It's easier to read.

[jenhagg]

Yeah that looks good, definitely more straightforward.