solving issue #1355 - Mixed dependencies in the Storage Clients module for the Data Layer

[ndricca]

PR to handle issue #1355

I have added also a pytest for S3StorageClient and moto to mock aws connection. I am not so experienced with Azure so I did not add a test on it.

[hadarsharon]

Thanks for implementing this @ndricca 🥇

[ndricca]

i noticed that @dokterbob said

I imagine storage_clients module in the data module with a base.py and files for each implementation

while I was still using the base.py from parent folder, so i splitted also this script moving the base method into storage_clients submodule

[dokterbob]

Can't wait to review this (prolly around Wednesday in the coming week), unless @willydouhard beats me to it!

[ndricca]

@dokterbob am i suppose to push also the poetry.lock file? I see fails refer to this. I ran all poetry tests in local and they worked

[gsornsen]

Oh, nice! Looking forward to trying this out and dropping the need for a supervisor to start up moto-server and create a local s3 bucket in my current implementation.

[dokterbob]

@dokterbob am i suppose to push also the poetry.lock file? I see fails refer to this. I ran all poetry tests in local and they worked

We use the lockfile to make sure we're all dev'ing in exactly the same environment, creating more reproducible results.

[ndricca]

@dokterbob am i suppose to push also the poetry.lock file? I see fails refer to this. I ran all poetry tests in local and they worked

We use the lockfile to make sure we're all dev'ing in exactly the same environment, creating more reproducible results.

I have merged incoming commits from main and pushed poetry.lock, now it should work

[dokterbob]

Good stuff! Looking forward to have this in, just some minor changes about file layout.

[dokterbob]

Please rename from azure_storage_client to simply azure - the import path already is sufficiently clear. If in code more clarity is needed one can always use aliased imports.

[dokterbob]

Same, please rename to merely s3.

[dokterbob]

Please, follow the same import path pattern in test layout. So /data/storage_clients/test_s3.py.

[dokterbob]

It doesn't seem changes to this file are relevant to this PR. Things like these can easily create merge conflicts!

Please git checkout main -- backend/tests/data/test_sql_alchemy.py this one. :)

[ndricca]

i did this, than changed other things and pushed but i see that this file is still here 🤔

[dokterbob]

Did you do git mv?

[ndricca]

no, can you please tell me what do you want me to do? a git revert to exclude it? please tell me the proper commands, i don't want to make noise in the repo.

moreover, i have also updated cypress/e2e/custom_data_layer/sql_alchemy.py since it uses the AzureStorageClient, is that ok or not?

[dokterbob]

Did you do git mv?