Skip to content

Commit

Permalink
Merge pull request #136 from SciCatProject/sftp-transfer
Browse files Browse the repository at this point in the history
Add SFTPFileTransfer
  • Loading branch information
jl-wynen authored Aug 28, 2023
2 parents 8e79551 + d75d420 commit 42b0509
Show file tree
Hide file tree
Showing 33 changed files with 1,549 additions and 72 deletions.
20 changes: 10 additions & 10 deletions docs/developer/testing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,22 +21,22 @@ In addition, the fakes are relatively complex and may diverge from the real impl

To mitigate these problems, the fake client is tested alongside a real client.
But to (mostly) avoid the downsides stated in the beginning, the real client is connected to a local SciCat server.
See ``tests/common/backend.py`` and ``tests/conftest.py`` for the concrete setup.
The backend is launched in a docker container based on the images provided by `scicatlive <https://github.com/SciCatProject/scicatlive>`_.
Tests in ``tests/client_tests.py`` are then run with both the fake and the real client to ensure that both produce the same results.
See ``src/testing/backend.py`` and ``tests/conftest.py`` for the concrete setup.
The backend is launched in a docker container with the image of the latest release of the SciCat backend.
Tests in ``tests/client`` are then run with both the fake and the real client to ensure that both produce the same results.

Use ``--backend-tests`` with ``pytest`` to run these tests.

SSH file transfer
-----------------
SFTP file transfer
------------------

Testing :class:`scitacean.transfer.ssh.SSHFileTransfer` requires an SSH server.
``tests/common/ssh_server.py`` contains helpers for managing one via Docker.
Tests can use it by depending on the ``ssh_fileserver`` fixture.
See the documentation in ``tests/common/ssh_server.py`` for how it works.
Testing :class:`scitacean.transfer.sftp.SFTPFileTransfer` requires an SFTP server.
``tests/common/sftp_server.py`` contains helpers for managing one via Docker.
Tests can use it by depending on the ``sftp_fileserver`` fixture.
See the documentation in ``tests/common/sftp_server.py`` for how it works.

Note that those tests may leave a small directory behind.
This is an issue with file ownership and permissions caused by making the Docker volumes writable.
It should not be a problem in practice as those files will only be in temporary directories which should get cleaned up at reboot.

Use ``--ssh-tests`` with ``pytest`` to run these tests.
Use ``--sftp-tests`` with ``pytest`` to run these tests.
3 changes: 3 additions & 0 deletions docs/release-notes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ Features

* File downloads now use the checksum algorithm stored in SciCat when possible.
So it is no longer necessary to specify it by hand in many cases.
* Added ``SFTPFileTransfer`` which is similar to ``SSHFileTransfer`` but relies only on SFTP.

Breaking changes
~~~~~~~~~~~~~~~~
Expand All @@ -58,6 +59,8 @@ Documentation
Deprecations
~~~~~~~~~~~~

* Deprecated ``SSHFileTransfer`` in favor of ``SFTPFileTransfer``.

Stability, Maintainability, and Testing
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
11 changes: 4 additions & 7 deletions docs/user-guide/downloading.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
"\n",
"```python\n",
"from scitacean import Client\n",
"from scitacean.transfer.ssh import SSHFileTransfer\n",
"from scitacean.transfer.sftp import SFTPFileTransfer\n",
"client = Client.from_token(url=\"https://scicat.ess.eu/api/v3\",\n",
" token=...,\n",
" file_transfer=SSHFileTransfer(\n",
" file_transfer=SFTPFileTransfer(\n",
" host=\"login.esss.dk\"\n",
" ))\n",
"```\n",
Expand All @@ -40,13 +40,10 @@
"</div>\n",
"\n",
"While the client itself is responsible for talking to SciCat, a `file_transfer` object is required to download data files.\n",
"Currently, only `SSHFileTransfer` is implemented.\n",
"It downloads / uploads files via SSH.\n",
"It will almost definitely change in the future!\n",
"Here, we use `SFTPFileTransfer` which downloads / uploads files via SFTP.\n",
"\n",
"The file transfer needs to authenticate separately from the SciCat connection.\n",
"It will ask for username and password on the command if needed.\n",
"But it is recommended to set up an SSH agent with a key and customize the `host` argument to point to the configured connection.\n",
"By default, it requires an SSH agent to be running an set up for the selected `host`.\n",
"\n",
"For the purposes of this guide, we don't want to connect to a real SciCat server in order to avoid the complications associated with that.\n",
"So we set up a fake client that only pretends to connect to SciCat and file servers.\n",
Expand Down
70 changes: 35 additions & 35 deletions docs/user-guide/testing.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
"They can be mixed and matched freely with the real client and file transfers.\n",
"But it is generally recommended to combine them.\n",
"\n",
"Secondly, SciCat servers and fileservers are managed by the [scicat_backend](../generated/modules/scitacean.testing.backend.fixtures.scicat_backend.rst) and [ssh_fileserver](../generated/modules/scitacean.testing.ssh.fixtures.ssh_fileserver.rst) pytest fixtures.\n",
"Secondly, SciCat servers and fileservers are managed by the [scicat_backend](../generated/modules/scitacean.testing.backend.fixtures.scicat_backend.rst) and [sftp_fileserver](../generated/modules/scitacean.testing.sftp.fixtures.sftp_fileserver.rst) pytest fixtures.\n",
"\n",
"First, create a test dataset and file."
]
Expand Down Expand Up @@ -554,23 +554,24 @@
"id": "97930d15-517a-4cea-bf2b-38ed700220b3",
"metadata": {},
"source": [
"## Local SSH fileserver\n",
"## Local SFTP fileserver\n",
"\n",
"[scitacean.testing.ssh](../generated/modules/scitacean.testing.ssh.rst) provides tools to set up an SSH server in a Docker container on the local machine.\n",
"It is primarily intended to be used via the [pytest](https://docs.pytest.org/) fixtures in [scitacean.testing.ssh.fixtures](../generated/modules/scitacean.testing.ssh.fixtures.rst).\n",
"[scitacean.testing.sftp](../generated/modules/scitacean.testing.sftp.rst) provides tools to set up an SFTP server in a Docker container on the local machine.\n",
"It is primarily intended to be used via the [pytest](https://docs.pytest.org/) fixtures in [scitacean.testing.sftp.fixtures](../generated/modules/scitacean.testing.sftp.fixtures.rst).\n",
"\n",
"The fixtures can configure, spin up, and seed an SSH server in a Docker container.\n",
"The fixtures can configure, spin up, and seed an SFTP server in a Docker container.\n",
"They also clean up after the test session by stopping the Docker container.\n",
"(Scritly speaking, the server is an SSH server but all users except root are restricted to SFTP.)\n",
"\n",
"Note the caveats in [scitacean.testing.ssh](../generated/modules/scitacean.testing.ssh.rst) about clean up and use of `pytest-xdist`.\n",
"Note the caveats in [scitacean.testing.sftp](../generated/modules/scitacean.testing.sftp.rst) about clean up and use of `pytest-xdist`.\n",
"\n",
"### Set up\n",
"\n",
"First, ensure that [Docker](https://www.docker.com/) is installed and running on your machine.\n",
"Then, configure pytest by\n",
"\n",
"- registering the fixtures and\n",
"- adding a command line option to enable ssh tests.\n",
"- adding a command line option to enable sftp tests.\n",
"\n",
"To this end, add the following in your `conftest.py`: (Or merge it into the setup for backend tests from above.)"
]
Expand All @@ -583,26 +584,26 @@
"outputs": [],
"source": [
"import pytest\n",
"from scitacean.testing.ssh import add_pytest_option as add_ssh_option\n",
"from scitacean.testing.sftp import add_pytest_option as add_sftp_option\n",
"\n",
"\n",
"pytest_plugins = (\n",
" \"scitacean.testing.ssh.fixtures\",\n",
" \"scitacean.testing.sftp.fixtures\",\n",
")\n",
"\n",
"def pytest_addoption(parser: pytest.Parser) -> None:\n",
" add_backend_option(parser)"
" add_sftp_option(parser)"
]
},
{
"cell_type": "markdown",
"id": "0d92f3dd-986c-4f2c-acec-00b6e55ae87b",
"metadata": {},
"source": [
"The SSH server will only be launched when the corresponding command line option is given.\n",
"By default, this is `--ssh-tests` but it can be changed via the `option` argument of `add_pytest_option`.\n",
"The SFTP server will only be launched when the corresponding command line option is given.\n",
"By default, this is `--sftp-tests` but it can be changed via the `option` argument of `add_pytest_option`.\n",
"\n",
"### Use SSH in tests\n",
"### Use SFTP in tests\n",
"\n",
"Tests that require the server can now request it as a fixture:"
]
Expand All @@ -614,7 +615,7 @@
"metadata": {},
"outputs": [],
"source": [
"def test_something_with_ssh(require_ssh_fileserver):\n",
"def test_something_with_sftp(require_sftp_fileserver):\n",
" # test something\n",
" ..."
]
Expand All @@ -624,12 +625,12 @@
"id": "3126d0d9-727a-4d55-aff1-6b31fd5df91f",
"metadata": {},
"source": [
"The `require_ssh_fileserver` fixture will ensure that the SSH server is running during the test.\n",
"If SSH tests have not been enabled by the command line option, the test will be skipped.\n",
"The `require_sftp_fileserver` fixture will ensure that the SFTP server is running during the test.\n",
"If SFTP tests have not been enabled by the command line option, the test will be skipped.\n",
"\n",
"Connecting to the server is not as straight forward as for the SciCat backend.\n",
"It requires passing a special `connect` function to the file transfer.\n",
"This can be done by requesting `ssh_connect_with_username_password`.\n",
"This can be done by requesting `sftp_connect_with_username_password`.\n",
"For example, the following opens a connection to the server to upload a file:"
]
},
Expand All @@ -640,23 +641,22 @@
"metadata": {},
"outputs": [],
"source": [
"from scitacean.transfer.ssh import SSHFileTransfer\n",
"from scitacean.transfer.sftp import SFTPFileTransfer\n",
"\n",
"def test_ssh_upload(\n",
" ssh_access,\n",
" ssh_connect_with_username_password,\n",
" require_ssh_fileserver,\n",
" ssh_data_dir,\n",
"def test_sftp_upload(\n",
" sftp_access,\n",
" sftp_connect_with_username_password,\n",
" require_sftp_fileserver,\n",
" sftp_data_dir,\n",
"):\n",
" ssh = SSHFileTransfer(host=ssh_access.host, port=ssh_access.port)\n",
" sftp = SFTPFileTransfer(host=sftp_access.host,\n",
" port=sftp_access.port,\n",
" connect=sftp_connect_with_username_password)\n",
" ds = Dataset(...)\n",
" with ssh.connect_for_upload(\n",
" dataset=ds,\n",
" connect=ssh_connect_with_username_password\n",
" ) as connection:\n",
" with sftp.connect_for_upload(dataset=ds) as connection:\n",
" # do upload\n",
" ...\n",
" # assert that the file has been copied to ssh_data_dir\n",
" # assert that the file has been copied to sftp_data_dir\n",
" ..."
]
},
Expand All @@ -666,17 +666,17 @@
"metadata": {},
"source": [
"Uploaded files are readable on the host.\n",
"So the test can read from `ssh_data_dir` to check if the upload succeeded.\n",
"So the test can read from `sftp_data_dir` to check if the upload succeeded.\n",
"This directory is mounted as `/data` on the server.\n",
"\n",
"Using an SSH file transfer with `Client` requires some extra steps.\n",
"An example is given by `test_client_with_ssh` in https://github.com/SciCatProject/scitacean/blob/main/tests/transfer/ssh_test.py.\n",
"It uses a subclass of `SSHFileTransfer` to pass `ssh_connect_with_username_password` to the connection as `Client` cannot do this itself. \n",
"Using an SFTP file transfer with `Client` requires some extra steps.\n",
"An example is given by `test_client_with_sftp` in https://github.com/SciCatProject/scitacean/blob/main/tests/transfer/sftp_test.py.\n",
"It uses a subclass of `SFTPFileTransfer` to pass `sftp_connect_with_username_password` to the connection as `Client` cannot do this itself.\n",
"\n",
"### Seed data\n",
"\n",
"The server's filesystem gets seeded with some files from https://github.com/SciCatProject/scitacean/tree/main/src/scitacean/testing/ssh/ssh_server_seed.\n",
"Those files are copied to `ssh_data_dir` on the host which is mounted to `/data/seed` on the server."
"The server's filesystem gets seeded with some files from https://github.com/SciCatProject/scitacean/tree/main/src/scitacean/testing/sftp/sftp_server_seed.\n",
"Those files are copied to `sftp_data_dir` on the host which is mounted to `/data/seed` on the server."
]
},
{
Expand Down
4 changes: 2 additions & 2 deletions docs/user-guide/uploading.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
"We connect to SciCat and a file server using a [Client](../generated/classes/scitacean.Client.rst):\n",
"```python\n",
"from scitacean import Client\n",
"from scitacean.transfer.ssh import SSHFileTransfer\n",
"from scitacean.transfer.sftp import SFTPFileTransfer\n",
"client = Client.from_token(url=\"https://scicat.ess.eu/api/v3\",\n",
" token=...,\n",
" file_transfer=SSHFileTransfer(\n",
" file_transfer=SFTPFileTransfer(\n",
" host=\"login.esss.dk\"\n",
" ))\n",
"```\n",
Expand Down
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ dynamic = ["version"]

[project.optional-dependencies]
ssh = ["fabric"]
sftp = ["paramiko"]
test = ["filelock", "hypothesis", "pyyaml"]

[tool.setuptools_scm]
Expand All @@ -60,6 +61,8 @@ filterwarnings = [
"error",
# Many tests don't set a checksum, so File raises this warning.
"ignore:Cannot check if local file:UserWarning",
# Internal deprecations.
"ignore:SSHFileTransfer is deprecated:scitacean.VisibleDeprecationWarning",
# From fabric / invoke
"ignore:_SixMetaPathImporter:ImportWarning",
"ignore:the imp module is deprecated in favour of importlib:DeprecationWarning",
Expand Down
1 change: 1 addition & 0 deletions requirements-pydantic2/base.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
email-validator
fabric
paramiko
pydantic == 2
python-dateutil
requests
63 changes: 63 additions & 0 deletions requirements-pydantic2/base.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# SHA1:2b55aaac9edd959da15c313d2f5f84f4e2cc5f2a
#
# This file is autogenerated by pip-compile-multi
# To update, run:
#
# pip-compile-multi
#
annotated-types==0.5.0
# via pydantic
bcrypt==4.0.1
# via paramiko
certifi==2023.7.22
# via requests
cffi==1.15.1
# via
# cryptography
# pynacl
charset-normalizer==3.2.0
# via requests
cryptography==41.0.3
# via paramiko
decorator==5.1.1
# via fabric
deprecated==1.2.14
# via fabric
dnspython==2.4.2
# via email-validator
email-validator==2.0.0.post2
# via -r requirements-pydantic2/base.in
fabric==3.2.1
# via -r requirements-pydantic2/base.in
idna==3.4
# via
# email-validator
# requests
invoke==2.2.0
# via fabric
paramiko==3.3.1
# via
# -r requirements-pydantic2/base.in
# fabric
pycparser==2.21
# via cffi
pydantic==2.0
# via -r requirements-pydantic2/base.in
pydantic-core==2.0.1
# via pydantic
pynacl==1.5.0
# via paramiko
python-dateutil==2.8.2
# via -r requirements-pydantic2/base.in
requests==2.31.0
# via -r requirements-pydantic2/base.in
six==1.16.0
# via python-dateutil
typing-extensions==4.7.1
# via
# pydantic
# pydantic-core
urllib3==2.0.4
# via requests
wrapt==1.15.0
# via deprecated
37 changes: 37 additions & 0 deletions requirements-pydantic2/test.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
# SHA1:cefe43dc5cb9d9d7430647ec8f8955a6f1576e53
#
# This file is autogenerated by pip-compile-multi
# To update, run:
#
# pip-compile-multi
#
-r base.txt
attrs==23.1.0
# via hypothesis
execnet==2.0.2
# via pytest-xdist
filelock==3.12.2
# via -r requirements-pydantic2/test.in
hypothesis==6.82.6
# via -r requirements-pydantic2/test.in
iniconfig==2.0.0
# via pytest
packaging==23.1
# via pytest
pluggy==1.3.0
# via pytest
pyfakefs==5.2.4
# via -r requirements-pydantic2/test.in
pytest==7.4.0
# via
# -r requirements-pydantic2/test.in
# pytest-randomly
# pytest-xdist
pytest-randomly==3.15.0
# via -r requirements-pydantic2/test.in
pytest-xdist==3.3.1
# via -r requirements-pydantic2/test.in
pyyaml==6.0.1
# via -r requirements-pydantic2/test.in
sortedcontainers==2.4.0
# via hypothesis
1 change: 1 addition & 0 deletions requirements/base.in
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
email-validator
fabric
paramiko
pydantic < 2 # autodoc_pydantic is not compatible with pydantic 2
python-dateutil
requests
6 changes: 4 additions & 2 deletions requirements/base.txt
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# SHA1:a579aa51495a128838b22e1692012f3918753a8c
# SHA1:f8c625c62cb057cead0d70b0a458c8aed4f43459
#
# This file is autogenerated by pip-compile-multi
# To update, run:
Expand Down Expand Up @@ -34,7 +34,9 @@ idna==3.4
invoke==2.2.0
# via fabric
paramiko==3.3.1
# via fabric
# via
# -r requirements/base.in
# fabric
pycparser==2.21
# via cffi
pydantic==1.10.12
Expand Down
Loading

0 comments on commit 42b0509

Please sign in to comment.