Fix wrong redis key (#1504)

* pylint will now output the error + where to look

* keep alive will now be correctly set
avoids creating corrupted keys

* adjusted a couple of missing splits

* refactored pylint assert test (maybe)

* added missing requirement for testing and dev

* removing 1 level of search depth

* installing in editable mode

* added missing dependency to catalog

* added depoendencies for service-library

* fixing paths

* added storage service missing requirements

* fixed naming, it is now clearer

* triggering a new build

* enhanced output

Co-authored-by: Andrei Neagu <[email protected]>
Co-authored-by: Odei Maiz <[email protected]>

Author: 3 people

packages/pytest-simcore/src/pytest_simcore/helpers/utils_pylint.py

@@ -0,0 +1,20 @@
+""" Helps with running Pylint tests on different modules """
+import subprocess
+
+
+AUTODETECT = 0
+
+
+def assert_pylint_is_passing(pylintrc, package_dir, number_of_jobs: int = AUTODETECT):
+    """Runs Pylint with given inputs. In case of error some helpful Pylint messages are displayed
+
+    This is used in different packages
+    """
+    command = f"pylint --jobs={number_of_jobs} --rcfile {pylintrc} -v {package_dir}".split(
+        " "
+    )
+    pipes = subprocess.Popen(command, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+    std_out, _ = pipes.communicate()
+    if pipes.returncode != 0:
+        print(f'>>>> Exit code "{pipes.returncode}"\n{std_out.decode("utf-8")}\n<<<<')
+        assert False, "Pylint failed with error, check this test's stdout to fix it"

packages/service-library/requirements/ci.txt

@@ -9,5 +9,8 @@
 # installs base + tests requirements
 -r _test.txt
 
+# installs this repo's packages
+../pytest-simcore/
+
 # current module
 .

packages/service-library/requirements/dev.txt

@@ -9,6 +9,9 @@
 # installs base + tests requirements
 -r _test.txt
 
+# installs this repo's packages
+-e ../pytest-simcore/
+
 # current module
 -e .
 

packages/service-library/tests/test_package.py

@@ -5,11 +5,11 @@
 
 import os
 import re
-import subprocess
 from pathlib import Path
 
 import pytest
 from servicelib.utils import is_osparc_repo_dir, search_osparc_repo_dir
+from pytest_simcore.helpers.utils_pylint import assert_pylint_is_passing
 
 
 @pytest.fixture
@@ -20,12 +20,7 @@ def pylintrc(osparc_simcore_root_dir):
 
 
 def test_run_pylint(pylintrc, package_dir):
-    try:
-        AUTODETECT = 0
-        cmd = f"pylint --jobs={AUTODETECT} --rcfile {pylintrc} -v {package_dir}".split()
-        assert subprocess.check_call(cmd) == 0
-    except subprocess.CalledProcessError as err:
-        pytest.fail("Linting error. Linter existed with code %d" % err.returncode)
+    assert_pylint_is_passing(pylintrc=pylintrc, package_dir=package_dir)
 
 
 def test_no_pdbs_in_place(package_dir):

services/api-gateway/requirements/ci.txt

@@ -9,5 +9,8 @@
 # installs base + tests requirements
 -r _test.txt
 
+# installs this repo's packages
+../../packages/pytest-simcore/
+
 # installs current package
 .

services/api-gateway/requirements/dev.txt

@@ -9,6 +9,9 @@
 # installs base + tests requirements
 -r _test.txt
 
+# installs this repo's packages
+-e ../../packages/pytest-simcore/
+
 # installs current package
 -e .
 

services/api-gateway/tests/unit/test_code_syntax.py

@@ -4,8 +4,9 @@
 
 import os
 import re
-import subprocess
 from pathlib import Path
+from pytest_simcore.helpers.utils_pylint import assert_pylint_is_passing
+
 
 import pytest
 
@@ -18,8 +19,7 @@ def pylintrc(osparc_simcore_root_dir):
 
 
 def test_run_pylint(pylintrc, package_dir):
-    cmd = "pylint --jobs 0 --rcfile {} -v {}".format(pylintrc, package_dir)
-    assert subprocess.check_call(cmd.split()) == 0
+    assert_pylint_is_passing(pylintrc=pylintrc, package_dir=package_dir)
 
 
 def test_no_pdbs_in_place(package_dir):

services/catalog/requirements/ci.txt

@@ -11,6 +11,7 @@
 
 # installs this repo's packages
 ../../packages/postgres-database
+../../packages/pytest-simcore/
 
 # installs current package
 .

services/catalog/requirements/dev.txt

@@ -11,6 +11,7 @@
 
 # installs this repo's packages
 -e ../../packages/postgres-database/
+-e ../../packages/pytest-simcore/
 
 # installs current package
 -e .

services/catalog/tests/unit/test_package.py

@@ -4,7 +4,7 @@
 
 import os
 import re
-import subprocess
+from pytest_simcore.helpers.utils_pylint import assert_pylint_is_passing
 from pathlib import Path
 
 import pytest
@@ -22,8 +22,7 @@ def pylintrc(project_slug_dir, osparc_simcore_root_dir):
 
 
 def test_run_pylint(pylintrc, package_dir):
-    cmd = "pylint --jobs 0 --rcfile {} -v {}".format(pylintrc, package_dir)
-    assert subprocess.check_call(cmd.split()) == 0
+    assert_pylint_is_passing(pylintrc=pylintrc, package_dir=package_dir)
 
 
 # FIXME: main entrypoint

services/sidecar/tests/unit/test_code_syntax.py

@@ -4,8 +4,8 @@
 
 import os
 import re
-import subprocess
 from pathlib import Path
+from pytest_simcore.helpers.utils_pylint import assert_pylint_is_passing
 
 import pytest
 
@@ -18,9 +18,7 @@ def pylintrc(osparc_simcore_root_dir):
 
 
 def test_run_pylint(pylintrc, package_dir):
-    cmd = "pylint --jobs 0 --rcfile {} -v {}".format(pylintrc, package_dir)
-    proc: subprocess.CompletedProcess = subprocess.run(cmd.split(), check=True)
-    assert proc.returncode == 0, f"pylint error: {proc.stdout}"
+    assert_pylint_is_passing(pylintrc=pylintrc, package_dir=package_dir)
 
 
 def test_no_pdbs_in_place(package_dir):
@@ -33,4 +31,4 @@ def test_no_pdbs_in_place(package_dir):
                 code = pypth.read_text()
                 found = MATCH.findall(code)
                 assert not found, "pbd.set_trace found in %s" % pypth
-        dirs[:] = [d for d in dirs if d not in EXCLUDE]
+        dirs[:] = [d for d in dirs if d not in EXCLUDE]

services/storage/requirements/ci.txt

@@ -14,6 +14,7 @@
 ../../packages/postgres-database/
 ../../packages/simcore-sdk/
 ../../packages/service-library/
+../../packages/pytest-simcore/
 
 # installs current package
 .

services/storage/requirements/dev.txt

@@ -14,6 +14,7 @@
 -e ../../packages/postgres-database/
 -e ../../packages/simcore-sdk/
 -e ../../packages/service-library
+-e ../../packages/pytest-simcore/
 
 # installs current package
 -e .

services/storage/tests/test_package.py

@@ -5,7 +5,7 @@
 # W0621: Redefining name ... from outer scope
 # pylint: disable=W0621
 
-import subprocess
+from pytest_simcore.helpers.utils_pylint import assert_pylint_is_passing
 
 import pytest
 
@@ -20,12 +20,7 @@ def pylintrc(osparc_simcore_root_dir):
 
 
 def test_run_pylint(pylintrc, package_dir):
-    try:
-        AUTODETECT = 0
-        cmd = f"pylint --jobs={AUTODETECT} --rcfile {pylintrc} -v {package_dir}".split()
-        assert subprocess.check_call(cmd) == 0
-    except subprocess.CalledProcessError as err:
-        pytest.fail("Linting error. Linter existed with code %d" % err.returncode)
+    assert_pylint_is_passing(pylintrc=pylintrc, package_dir=package_dir)
 
 
 def test_main(here):  # pylint: disable=unused-variable

services/web/server/src/simcore_service_webserver/resource_manager/websocket_manager.py

@@ -76,6 +76,13 @@ async def remove_socket_id(self) -> None:
             self._resource_key(), False, get_service_deletion_timeout(self.app)
         )
 
+    async def set_heartbeat(self) -> None:
+        """Refreshes heartbeat """
+        registry = get_registry(self.app)
+        await registry.set_key_alive(
+            self._resource_key(), False, get_service_deletion_timeout(self.app)
+        )
+
     async def find_socket_ids(self) -> List[str]:
         log.debug(
             "user %s/tab %s finding %s from registry...",

services/web/server/src/simcore_service_webserver/socketio/handlers.py

@@ -17,7 +17,7 @@
 from socketio.exceptions import ConnectionRefusedError as SocketIOConnectionError
 
 from ..login.decorators import RQT_USERID_KEY, login_required
-from ..resource_manager.websocket_manager import managed_resource, get_registry
+from ..resource_manager.websocket_manager import managed_resource
 from ..resource_manager.config import get_service_deletion_timeout
 from .config import get_socket_server
 from .handlers_utils import register_socketio_handler
@@ -57,7 +57,7 @@ async def connect(sid: str, environ: Dict, app: web.Application) -> bool:
     log.info("Sending set_heartbeat_emit_interval with %s", emit_interval)
 
     user_id = request.get(RQT_USERID_KEY, ANONYMOUS_USER_ID)
-    await post_messages(app, user_id, {'set_heartbeat_emit_interval': emit_interval})
+    await post_messages(app, user_id, {"set_heartbeat_emit_interval": emit_interval})
 
     return True
 
@@ -147,6 +147,7 @@ async def disconnect(sid: str, app: web.Application) -> None:
                 str(socketio_session),
             )
 
+
 @register_socketio_handler
 async def client_heartbeat(sid: str, _: Any, app: web.Application) -> None:
     """JS client invokes this handler to signal its presence.
@@ -161,7 +162,10 @@ async def client_heartbeat(sid: str, _: Any, app: web.Application) -> None:
     """
     sio = get_socket_server(app)
     async with sio.session(sid) as socketio_session:
-        registry = get_registry(app)
-        await registry.set_key_alive(
-            socketio_session, False, get_service_deletion_timeout(app)
-        )
+        if "user_id" not in socketio_session:
+            return
+
+        user_id = socketio_session["user_id"]
+        client_session_id = socketio_session["client_session_id"]
+        with managed_resource(user_id, client_session_id, app) as rt:
+            await rt.set_heartbeat()

services/web/server/tests/unit/test_package.py

@@ -5,7 +5,7 @@
 # pylint:disable=redefined-outer-name
 
 import pytest
-import subprocess
+from pytest_simcore.helpers.utils_pylint import assert_pylint_is_passing
 import os
 import re
 
@@ -22,12 +22,7 @@ def pylintrc(osparc_simcore_root_dir):
 
 
 def test_run_pylint(pylintrc, package_dir):
-    try:
-        AUTODETECT = 0
-        cmd = f"pylint --jobs={AUTODETECT} --rcfile {pylintrc} -v {package_dir}".split()
-        assert subprocess.check_call(cmd) == 0
-    except subprocess.CalledProcessError as err:
-        pytest.fail("Linting error. Linter existed with code %d" % err.returncode)
+    assert_pylint_is_passing(pylintrc=pylintrc, package_dir=package_dir)
 
 
 def test_main(here):  # pylint: disable=unused-variable