From 5e47193c84c3c87be7195b038f3e9865547b64ec Mon Sep 17 00:00:00 2001 From: "Budnick, Leon" Date: Fri, 22 Nov 2024 11:34:18 +0100 Subject: [PATCH 1/6] only subscribe if actions are available --- CHANGELOG.md | 1 + src/sdc11073/sdcclient/sdcclientimpl.py | 15 ++++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e0b7d80e..ed4bfaea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - accessing a multikey may lead to IndexError [#359](https://github.com/Draegerwerk/sdc11073/issues/359) - fixed a bug where `log_prefix` can only be a string [#393](https://github.com/Draegerwerk/sdc11073/issues/393) +- only send subscribe request if there are actions to subscribe to [#402](https://github.com/Draegerwerk/sdc11073/issues/402) ## [1.3.2] - 2024-03-18 diff --git a/src/sdc11073/sdcclient/sdcclientimpl.py b/src/sdc11073/sdcclient/sdcclientimpl.py index f01e2c25..adb27d16 100644 --- a/src/sdc11073/sdcclient/sdcclientimpl.py +++ b/src/sdc11073/sdcclient/sdcclientimpl.py @@ -408,13 +408,14 @@ def startAll(self, notSubscribedActions=None, subscriptionsCheckInterval=None, a subscribe_actions = set(available_actions) - notSubscribedActionsSet if not subscribe_periodic_reports: subscribe_actions -= set(periodic_actions) - try: - self._subscribe(dpwsHosted, subscribe_actions, - self._onAnyStateEventReport) - except Exception as ex: - self.all_subscribed = False # => do not log errors when mdib versions are missing in notifications - self._logger.error('startAll: could not subscribe: error = {}, actions= {}', - traceback.format_exc(), subscribe_actions) + if subscribe_actions: + try: + self._subscribe(dpwsHosted, subscribe_actions, + self._onAnyStateEventReport) + except Exception as ex: + self.all_subscribed = False # => do not log errors when mdib versions are missing in notifications + self._logger.error('startAll: could not subscribe: error = {}, actions= {}', + traceback.format_exc(), subscribe_actions) # connect self.isConnected observable to allSubscriptionsOkay observable in subscriptionsmanager def setIsConnected(isOk): From 57b55ff5b475aac101ff0701302f69be53f9e9d9 Mon Sep 17 00:00:00 2001 From: "Budnick, Leon" Date: Fri, 22 Nov 2024 11:42:55 +0100 Subject: [PATCH 2/6] update workflows --- .github/workflows/build.yml | 16 ++++++++-------- .github/workflows/publish.yml | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 483b8566..51043bbb 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -18,10 +18,10 @@ jobs: steps: - name: Check out Git repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: "3.12" @@ -43,7 +43,7 @@ jobs: echo TARGZ=$(echo dist/*.tar.gz) >> $GITHUB_OUTPUT - name: Archive package - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 with: name: distributions path: dist @@ -62,15 +62,15 @@ jobs: steps: - name: Check out Git repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - name: Download Artifact - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: distributions path: dist @@ -99,7 +99,7 @@ jobs: --log-file pytest_${{ matrix.os }}_py${{ matrix.python-version }}_${{ env.SUFFIX }}.log - name: Archive test result - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 if: success() || failure() # upload artifacts also if test stage failed with: name: unittest_reports @@ -107,7 +107,7 @@ jobs: retention-days: 5 - name: Archive test log - uses: actions/upload-artifact@v3 + uses: actions/upload-artifact@v4 if: success() || failure() # upload artifacts also if test stage failed with: name: pytest_logs diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 7d342b30..3d96f16f 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -20,7 +20,7 @@ jobs: steps: - name: Download Artifact - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: distributions path: dist/ @@ -46,16 +46,16 @@ jobs: steps: - name: Check out Git repository - uses: actions/checkout@v3 + uses: actions/checkout@v4 - name: Download Build - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: distributions path: dist - name: Download Unittest report - uses: actions/download-artifact@v3 + uses: actions/download-artifact@v4 with: name: unittest_reports path: pytest_reports/ From e01d8cab092ef544e847f3f4309c7fea8149dc9c Mon Sep 17 00:00:00 2001 From: "Budnick, Leon" Date: Tue, 26 Nov 2024 09:04:43 +0100 Subject: [PATCH 3/6] fix Conflict: an artifact with this name already exists on the workflow run --- .github/workflows/build.yml | 4 ++-- .github/workflows/publish.yml | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 51043bbb..b2770a9b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -102,7 +102,7 @@ jobs: uses: actions/upload-artifact@v4 if: success() || failure() # upload artifacts also if test stage failed with: - name: unittest_reports + name: unittest_report_${{ matrix.os }}_py${{ matrix.python-version }}_${{ env.SUFFIX }}.html path: pytest_reports/unittest_report_${{ matrix.os }}_py${{ matrix.python-version }}_${{ env.SUFFIX }}.html retention-days: 5 @@ -110,7 +110,7 @@ jobs: uses: actions/upload-artifact@v4 if: success() || failure() # upload artifacts also if test stage failed with: - name: pytest_logs + name: pytest_${{ matrix.os }}_py${{ matrix.python-version }}_${{ env.SUFFIX }}.log path: pytest_${{ matrix.os }}_py${{ matrix.python-version }}_${{ env.SUFFIX }}.log retention-days: 5 diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 3d96f16f..76f15110 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -57,8 +57,9 @@ jobs: - name: Download Unittest report uses: actions/download-artifact@v4 with: - name: unittest_reports + pattern: unittest_report_* path: pytest_reports/ + merge-multiple: true - name: Zip unittest reports uses: vimtor/action-zip@v1.1 From 5ba7f3cec16d86a08c7b7198d6fa1b9639f2ed16 Mon Sep 17 00:00:00 2001 From: "Budnick, Leon" Date: Thu, 23 Jan 2025 11:19:53 +0100 Subject: [PATCH 4/6] added unittest --- src/sdc11073/sdcclient/sdcclientimpl.py | 18 ++++++++++-------- tests/test_client_device.py | 8 ++++++++ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/src/sdc11073/sdcclient/sdcclientimpl.py b/src/sdc11073/sdcclient/sdcclientimpl.py index adb27d16..ec04fa83 100644 --- a/src/sdc11073/sdcclient/sdcclientimpl.py +++ b/src/sdc11073/sdcclient/sdcclientimpl.py @@ -408,14 +408,16 @@ def startAll(self, notSubscribedActions=None, subscriptionsCheckInterval=None, a subscribe_actions = set(available_actions) - notSubscribedActionsSet if not subscribe_periodic_reports: subscribe_actions -= set(periodic_actions) - if subscribe_actions: - try: - self._subscribe(dpwsHosted, subscribe_actions, - self._onAnyStateEventReport) - except Exception as ex: - self.all_subscribed = False # => do not log errors when mdib versions are missing in notifications - self._logger.error('startAll: could not subscribe: error = {}, actions= {}', - traceback.format_exc(), subscribe_actions) + if not subscribe_actions: + self._logger.warning('startAll: no actions to subscribe for service_id = {}', service_id) + continue + try: + self._subscribe(dpwsHosted, subscribe_actions, + self._onAnyStateEventReport) + except Exception as ex: + self.all_subscribed = False # => do not log errors when mdib versions are missing in notifications + self._logger.error('startAll: could not subscribe: error = {}, actions= {}', + traceback.format_exc(), subscribe_actions) # connect self.isConnected observable to allSubscriptionsOkay observable in subscriptionsmanager def setIsConnected(isOk): diff --git a/tests/test_client_device.py b/tests/test_client_device.py index 5a1116c4..40efbf7f 100644 --- a/tests/test_client_device.py +++ b/tests/test_client_device.py @@ -22,6 +22,7 @@ from sdc11073 import namespaces from sdc11073 import observableproperties from sdc11073 import pmtypes +from sdc11073.definitions_sdc import SDC_v1_Definitions from sdc11073.location import SdcLocation from sdc11073.mdib import ClientMdibContainer from sdc11073.mdib import clientmdib @@ -1756,6 +1757,13 @@ def test_invalid_request(self): else: self.assertTrue(False, 'HTTPReturnCodeError not raised') + def test_subscribe_not_all(self): + self.sdcClient_Final.stopAll() + with mock.patch.object(self.sdcClient_Final._logger, 'warning') as mock_logger: + self.sdcClient_Final.startAll(notSubscribedActions=[SDC_v1_Definitions.Actions.OperationInvokedReport]) + self.assertFalse(self.sdcClient_Final.all_subscribed) + mock_logger.assert_any_call('startAll: no actions to subscribe for service_id = {}', 'SetService') + class Test_DeviceCommonHttpServer(unittest.TestCase): @classmethod From 29270e1ae77b9705ed486fb0ec7c6719204ee095 Mon Sep 17 00:00:00 2001 From: "Budnick, Leon" Date: Tue, 28 Jan 2025 10:12:50 +0100 Subject: [PATCH 5/6] test step uses ubuntu-22.04 instead of latest --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index b2770a9b..6a014caf 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -55,7 +55,7 @@ jobs: strategy: matrix: python-version: [ "3.7", "3.8" , "3.9", "3.10", "3.11", "3.12" ] - os: [ ubuntu-latest, windows-latest ] + os: [ ubuntu-22.04, windows-latest ] distribution: [ "${{ needs.build.outputs.WHL }}", "${{ needs.build.outputs.TARGZ }}" ] runs-on: ${{ matrix.os }} From e1e05905cb1f045bf83f8f1db1ae438a0592511e Mon Sep 17 00:00:00 2001 From: "Budnick, Leon" Date: Tue, 28 Jan 2025 11:45:20 +0100 Subject: [PATCH 6/6] test step uses ubuntu-22.04 instead of latest --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6a014caf..b36dd17f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -79,7 +79,7 @@ jobs: run: pip install ${{ matrix.distribution }}[test] - name: Detect suffix on ubuntu - if: ${{ matrix.os == 'ubuntu-latest' }} + if: ${{ matrix.os == 'ubuntu-22.04' }} run: | echo SUFFIX=$(python -c "import pathlib;p=pathlib.Path('${{ matrix.distribution }}');print('whl') if p.suffix=='.whl' else print('tar_gz')") >> $GITHUB_ENV