Enhancement/Extend OpenRemote Client with service registration mechanism #40

dominiquekleeven · 2025-07-04T10:15:41Z

Changes

Extracted the OpenRemoteClient source from the base project to a separate package, managed via uv workspaces.
Refactored the OpenRemoteClient structure to use nested classes based on domain (e.g., assets, realms), resulting in a clearer and more intuitive API.
Added service.register for registering a service/microservice
Added service.deregister for deregistering a service/microservice
Added service.heartbeat for manually sending heartbeat pings
Added a scheduler (service_registrar.py) to the OpenRemoteClient for registering a service and periodically sending a heartbeat the registration alive

Other

Added additional test coverage for the OpenRemoteClient.

…tching based on issuer + configured OR URL.

…ient dependency

…directly to prevent inconsistent routing behaviour

…ger config

Co-authored-by: Rich Turner <7072278+richturner@users.noreply.github.com>

…chunking for datapoint retrieval Deprecated cutoff_timestamp

…nremote-client-service-register

…via the service backend Couple technical notes: - Front-end initialises the manager.rest.api in the app layout after authentication has succeeded - Auth service also adds the interceptor to Axios so that the manager.rest.api can use the token - Removed proxy logic from service backend, so api routes, tests and service methods.

…an in the async event loop It is because the underlying calls are synchronous by nature, in FastAPI endpoints marked with async are always ran in the eventloop even when blocking, when non async it attempts to coalasce and run requests in threads

…nremote-client-service-register

…h detection

…nremote-client-service-register

…selves

Copilot

Pull Request Overview

Copilot reviewed 60 out of 66 changed files in this pull request and generated 6 comments.

Files not reviewed (1)

frontend/package-lock.json: Language not supported

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-16T08:46:06Z

packages/openremote_client/src/openremote_client/models.py

+        x: The timestamp of the data point in milliseconds.
        y: The value of the data point.


The docstring comment should be consistent with the field name. Since the field is named 'x', the comment should refer to 'x' rather than using 'The timestamp of the data point'.

Suggested change

x: The timestamp of the data point in milliseconds.

y: The value of the data point.

`x`: The timestamp of the data point in milliseconds.

`y`: The value of the data point.

Copilot · 2025-09-16T08:46:06Z

src/service_ml_forecast/models/model_config.py

+    cutoff_timestamp: int | None = Field(
+        default=None,
+        description="Deprecated, use training_data_period instead.",
+        deprecated=True,
    )


The deprecated cutoff_timestamp field should include information about when it will be removed to help users plan migration. Consider adding a deprecation timeline like 'Deprecated since v1.1, will be removed in v2.0'.

Copilot · 2025-09-16T08:46:06Z

frontend/src/components/custom-duration-input.ts

+        const match = /P(?:T)?(\d+)([HMDWMOY]+)/.exec(duration);
        return match ? parseInt(match[1], 10) : null;
    }

    // Extract the unit from the ISO 8601 Duration string
    getUnitFromDuration(duration: string): TimeDurationUnit | null {
-        const match = /PT(\d+)([HM])/.exec(duration);
-        return match ? (match[2] as TimeDurationUnit) : null;
+        const match = /P(?:T)?(\d+)([HMDWMOY]+)/.exec(duration);


The regex pattern [HMDWMOY]+ allows multiple time units to be combined (e.g., 'HM', 'DWY'), which may not represent valid ISO 8601 durations. Consider using a more restrictive pattern like ([HMDWY]) to match only single valid time units.

Copilot · 2025-09-16T08:46:07Z

src/service_ml_forecast/middlewares/keycloak/middleware.py

+    # Construct JWKS URL using the provided keycloak_url
+    jwks_url = f"{issuer}{JWKS_ENDPOINT_PATH}"
+
+    try:


Using ENV.ML_VERIFY_SSL to control SSL verification could be a security risk if set to False in production. Consider adding a warning or restricting this to development environments only.

Suggested change

try:

try:

# Restrict disabling SSL verification to development environments only

if not ENV.ML_VERIFY_SSL:

# Check for environment indicator; default to 'production' if not present

environment = getattr(ENV, "ENVIRONMENT", "production").lower()

if environment not in ("development", "dev", "test", "testing"):

logger.error(

"SSL verification is disabled (ENV.ML_VERIFY_SSL=False) in a non-development environment (%s). This is a security risk.",

environment,

)

raise HTTPException(

status_code=HTTPStatus.INTERNAL_SERVER_ERROR,

detail="SSL verification cannot be disabled in production environments.",

)

else:

logger.warning(

"SSL verification is disabled (ENV.ML_VERIFY_SSL=False) in %s environment. This should only be used for local development or testing.",

environment,

)

Copilot · 2025-09-16T08:46:07Z

src/service_ml_forecast/common/time_util.py

+        # So we need to handle the conversion to seconds manually
+        if isinstance(parsed, Duration):
+            seconds = int(parsed.months * 30 * 24 * 60 * 60)
+            seconds += int(parsed.years * 365 * 24 * 60 * 60)  # Not taking into account leap years is OK


The approximation of months as 30 days and years as 365 days could lead to inaccuracies over time. Consider documenting this limitation more prominently or using a more accurate calendar-aware calculation library.

Copilot · 2025-09-16T08:46:07Z

packages/openremote_client/src/openremote_client/service_registrar.py

+        if not self.registered or not self.instance_id:
+            logger.warning("Cannot send heartbeat - service not registered, attempting re-registration")
+            self._register_service()
+            return


The automatic re-registration on failed heartbeat could lead to infinite retry loops if registration consistently fails. Consider implementing exponential backoff or maximum retry limits to prevent resource exhaustion.

…nremote-client-service-register

…e-register" This reverts commit 95c9591, reversing changes made to c4edea6.

dominiquekleeven · 2025-09-18T17:49:13Z

Replaced with new PR with a clean commit history #42

dominiquekleeven and others added 30 commits May 14, 2025 09:40

Clean commit history from feature/keycloak-middleware

331dfe8

Update pyproject.toml

6c122de

Construct valid issuer list based on OR realms rather than pattern ma…

128fc3a

…tching based on issuer + configured OR URL.

Additional comment

a4b7b07

Increase JWKS cache as its not tied to the expiry interval

6dfc19d

Unreachable code

76fc179

Minor tweaks, use both issuer and kid as cache key for JWKS.

b4c5c1d

Added TODO comment with issue link for RBAC improvements

aea9d25

Additional logging

75c8b76

Fix failing test due to model changes

2150eec

Login and logout are promises, should be awaited

ada37e1

Update app-layout.ts

098f611

Proxy service was unnecessary, we can just override the OpenRemoteCl…

a923798

…ient dependency

Use vaadin router commands for redirect rather than using the router …

66fb3d1

…directly to prevent inconsistent routing behaviour

Don't expect an optional property in the token payload

4ccfd65

Update pages-config-editor.ts

8f075b0

Increase timeout for HTTPX to accomodate slower requests

a2e0086

Fix timeout on proxy client

365f31b

Update openremote_proxy_client.py

a3c95dc

Minor refactor to OR client to properly support realm users

fced7b4

Fix exception on retrieving realm config when realm isn't in the mana…

bb6a3fd

…ger config

Update frontend/src/services/api-service.ts

11be849

Co-authored-by: Rich Turner <7072278+richturner@users.noreply.github.com>

Address misleading function name, uses a more explicit name now

e6d4e38

Merge branch 'main' into feature/keycloak-middleware-clean

944cb0d

Minor refactor to openremote client before extraction

bd2bb47

Use ISO8601 period for training dataset (regressors + target), added …

a8fbd08

…chunking for datapoint retrieval Deprecated cutoff_timestamp

Fix prettier formatting issue

3819b3d

Merge branch 'feature/keycloak-middleware-clean' into enhancement/ope…

ece85b9

…nremote-client-service-register

WIP tbd on registration mechanism location

3072bab

dominiquekleeven added 18 commits August 11, 2025 07:58

Merge branch 'feature/keycloak-middleware-clean' into enhancement/ope…

155f5c0

…nremote-client-service-register

Adjusted registration logic + role changes

b80f59c

Fix failing tests due to changed service object

32fd3f9

Remove unused param

f129442

Update middleware.py

bb9a48f

API changes

a793b27

Remove the logging prefix override, and clean up regex for bundle pat…

4a18dfe

…h detection

Merge branch 'feature/keycloak-middleware-clean' into enhancement/ope…

df21082

…nremote-client-service-register

Format

e255c5c

Format

21a5484

Support additional service fields

67d81b9

Unnecessary since these tests were covered by the forecast tests them…

437a394

…selves

Unnecessary since these tests were covered by the forecast tests them…

6b4e2fc

…selves

Only include the roles that are actually usable for services

22aea98

Fix failing test and format

880395c

Update route_exception_handlers.py

ff8a0fb

wborn requested a review from Copilot September 16, 2025 08:42

Copilot AI reviewed Sep 16, 2025

View reviewed changes

dominiquekleeven added 6 commits September 16, 2025 11:07

instanceId is now an integer

78bad1e

Merge branch 'feature/keycloak-middleware-clean' into enhancement/ope…

17a36d0

…nremote-client-service-register

Fix test failure and typing changes for registration

fe117c8

ClientRoles duplicate

c4edea6

Merge branch 'main' into enhancement/openremote-client-service-register

95c9591

Revert "Merge branch 'main' into enhancement/openremote-client-servic…

3862e03

…e-register" This reverts commit 95c9591, reversing changes made to c4edea6.

dominiquekleeven closed this Sep 18, 2025

dominiquekleeven deleted the enhancement/openremote-client-service-register branch September 18, 2025 17:49

dominiquekleeven mentioned this pull request Sep 18, 2025

Extend OpenRemote Client with service registration #42

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Enhancement/Extend OpenRemote Client with service registration mechanism #40

Enhancement/Extend OpenRemote Client with service registration mechanism #40

Uh oh!

dominiquekleeven commented Jul 4, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

Copilot AI Sep 16, 2025

Uh oh!

dominiquekleeven commented Sep 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		x: The timestamp of the data point in milliseconds.
		y: The value of the data point.

-    try:
+    try:
+        # Restrict disabling SSL verification to development environments only
+        if not ENV.ML_VERIFY_SSL:
+            # Check for environment indicator; default to 'production' if not present
+            environment = getattr(ENV, "ENVIRONMENT", "production").lower()
+            if environment not in ("development", "dev", "test", "testing"):
+                logger.error(
+                    "SSL verification is disabled (ENV.ML_VERIFY_SSL=False) in a non-development environment (%s). This is a security risk.",
+                    environment,
+                )
+                raise HTTPException(
+                    status_code=HTTPStatus.INTERNAL_SERVER_ERROR,
+                    detail="SSL verification cannot be disabled in production environments.",
+                )
+            else:
+                logger.warning(
+                    "SSL verification is disabled (ENV.ML_VERIFY_SSL=False) in %s environment. This should only be used for local development or testing.",
+                    environment,
+                )

Enhancement/Extend OpenRemote Client with service registration mechanism #40

Enhancement/Extend OpenRemote Client with service registration mechanism #40

Uh oh!

Conversation

dominiquekleeven commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Other

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

dominiquekleeven commented Sep 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dominiquekleeven commented Jul 4, 2025 •

edited

Loading