Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for config file locking #1719

Merged
merged 31 commits into from
Aug 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
a869d21
using filelock package for platform independency in locking the confi…
kessler-frost Jul 4, 2023
d5449a4
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 4, 2023
80ad2f5
updated changelog
kessler-frost Jul 4, 2023
9936952
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 4, 2023
1484932
trying to prevent the deadlock
kessler-frost Jul 4, 2023
99cc228
going backwards to check failure point
kessler-frost Jul 4, 2023
767b53b
fixing test
kessler-frost Jul 4, 2023
b195bc4
retracing
kessler-frost Jul 4, 2023
490439a
retracing
kessler-frost Jul 4, 2023
31c1237
disabling test
kessler-frost Jul 4, 2023
762985a
Merge branch 'develop' into fix-config-file-lock
kessler-frost Jul 6, 2023
1db7fb9
trying to prevent the deadlock
kessler-frost Jul 6, 2023
608d119
disabling some tests temporarily
kessler-frost Jul 6, 2023
0161b03
disabling some tests temporarily
kessler-frost Jul 6, 2023
4f18e29
disabling some tests temporarily
kessler-frost Jul 6, 2023
bb9cdc6
enabling the tests
kessler-frost Jul 6, 2023
01f4d94
cleaning up
kessler-frost Jul 6, 2023
a1f911e
added filelock to requirements
kessler-frost Jul 10, 2023
e6ae455
Merge branch 'develop' into fix-config-file-lock
kessler-frost Jul 11, 2023
2bbd280
Update CHANGELOG.md
kessler-frost Jul 11, 2023
21116a5
Merge branch 'develop' into fix-config-file-lock
kessler-frost Jul 20, 2023
9d865ca
retracted tests/requirements.txt changes
kessler-frost Jul 20, 2023
24dbbd2
fixed test and using >= in tests/requirements now
kessler-frost Jul 20, 2023
9ea0023
Merge branch 'develop' into fix-config-file-lock
kessler-frost Jul 20, 2023
366d51c
updated changelog and skipping remote fs test
kessler-frost Jul 21, 2023
90de77c
Merge branch 'develop' into fix-config-file-lock
kessler-frost Jul 28, 2023
1c13dab
fixing test
kessler-frost Jul 28, 2023
848e6bc
adding test
kessler-frost Jul 31, 2023
0e338ed
adding test
kessler-frost Jul 31, 2023
863a7e9
Merge branch 'develop' into fix-config-file-lock
kessler-frost Jul 31, 2023
97c7347
Merge branch 'develop' of github.com:AgnostiqHQ/covalent into fix-con…
wjcunningham7 Aug 8, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### Fixed

- Using `filelock` package now for platform independent file locking of config file. This should fix the failing tests as well as improve compatibility with Windows.
- When stopping the server, we send the proper `SIGINT` signal to uvicorn instead of `SIGKILL` which allows the second part of the FastAPI `lifespan` to execute properly.
- Fixed the outstanding incompatibities between front-end data layer and a postgres database
- Reverted file-lock changes
Expand Down
21 changes: 11 additions & 10 deletions covalent/_shared_files/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#
# Relief from the License may be granted by purchasing a commercial license.

import fcntl

import os
import shutil
from dataclasses import asdict
Expand All @@ -27,6 +27,7 @@
from pathlib import Path
from typing import Any, Dict, List, Optional, Union

import filelock
import toml

"""Configuration manager."""
Expand Down Expand Up @@ -109,16 +110,16 @@ def update_nested_dict(old_dict, new_dict, override_existing: bool = True):
else:
old_dict.setdefault(key, value)

with open(self.config_file, "r+") as f:
fcntl.lockf(f, fcntl.LOCK_EX)
file_config = toml.load(f)
with filelock.FileLock(f"{self.config_file}.lock", timeout=1):
with open(self.config_file, "r+") as f:
file_config = toml.load(f)

update_nested_dict(self.config_data, file_config)
if new_entries:
update_nested_dict(self.config_data, new_entries, override_existing)
update_nested_dict(self.config_data, file_config)
if new_entries:
update_nested_dict(self.config_data, new_entries, override_existing)

# Writing it back to the file
self.write_config()
# Writing it back to the file
self.write_config()

def read_config(self) -> None:
"""
Expand All @@ -143,8 +144,8 @@ def write_config(self) -> None:
Returns:
None
"""

with open(self.config_file, "w") as f:
fcntl.lockf(f, fcntl.LOCK_EX)
toml.dump(self.config_data, f)

def purge_config(self) -> None:
Expand Down
1 change: 1 addition & 0 deletions requirements-client.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ aiofiles>=0.8.0
aiohttp>=3.8.1
cloudpickle>=2.0.0
dask[distributed]>=2022.6.0
filelock>=3.12.2
furl>=2.1.3
networkx>=2.8.6
requests>=2.24.0
Expand Down
1 change: 1 addition & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ click>=8.1.3
cloudpickle>=2.0.0
dask[distributed]>=2022.6.0
fastapi>=0.93.0
filelock>=3.12.2
furl>=2.1.3
natsort>=8.4.0
networkx>=2.8.6
Expand Down
26 changes: 24 additions & 2 deletions tests/covalent_tests/shared_files/config_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,12 +219,34 @@ def test_write_config(mocker):
cm = ConfigManager()
toml_dump_mock = mocker.patch("covalent._shared_files.config.toml.dump")
open_mock = mocker.patch("covalent._shared_files.config.open")
lock_mock = mocker.patch("fcntl.lockf")
mock_file = open_mock.return_value.__enter__.return_value
cm.write_config()
toml_dump_mock.assert_called_once_with(cm.config_data, mock_file)
open_mock.assert_called_once_with(cm.config_file, "w")
lock_mock.assert_called_once()


def test_update_config(mocker):
"""Test the update_config method for config manager."""

cm = ConfigManager()

cm.config_file = "mock_config_file"
cm.config_data = {"mock_section": {"mock_dir": "initial_value"}}
# Cannot mock `update_nested_dict`` since it's defined within the function

mock_filelock = mocker.patch("covalent._shared_files.config.filelock.FileLock")
mock_open = mocker.patch("covalent._shared_files.config.open")
mock_toml_load = mocker.patch("covalent._shared_files.config.toml.load")

cm.write_config = mocker.Mock()

cm.update_config()

mock_filelock.assert_called_once_with("mock_config_file.lock", timeout=1)
mock_open.assert_called_once_with("mock_config_file", "r+")
mock_toml_load.assert_called_once_with(mock_open.return_value.__enter__.return_value)

cm.write_config.assert_called_once()


def test_config_manager_set(mocker):
Expand Down
2 changes: 2 additions & 0 deletions tests/functional_tests/docs_how_to_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@
"creating_custom_executors.ipynb",
"file_transfers_to_from_azure_blob.ipynb",
"file_transfers_to_from_gcp_storage.ipynb",
"file_transfers_for_workflows_to_remote.ipynb",
"file_transfers_to_remote.ipynb",
]


Expand Down
35 changes: 17 additions & 18 deletions tests/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,18 +1,17 @@
boto3==1.26.110
detect-secrets==1.3.0
flake8==5.0.4
httpx==0.24.1
isort==5.10.1
locust==2.11.0
mock==4.0.3
nbconvert==6.5.1
pennylane==0.25.1
pre-commit==2.20.0
pytest==7.1.3
pytest-asyncio==0.21.0
pytest-cov==3.0.0
pytest-mock==3.8.2
pytest-rerunfailures==10.2
pytest_asyncio==0.21.0
scikit-image==0.19.1
scikit-learn==1.2.2
boto3>=1.26.110
detect-secrets>=1.3.0
flake8>=5.0.4
httpx>=0.24.1
isort>=5.10.1
locust>=2.11.0
mock>=4.0.3
nbconvert>=6.5.1
pennylane>=0.25.1
pre-commit>=2.20.0
pytest>=7.1.3
pytest-asyncio>=0.21.0
pytest-cov>=3.0.0
pytest-mock>=3.8.2
pytest-rerunfailures>=10.2
scikit-image>=0.19.1
scikit-learn>=1.2.2
Loading