-
Notifications
You must be signed in to change notification settings - Fork 60
Feature impr/digital twin registry #859
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
base: main
Are you sure you want to change the base?
Feature impr/digital twin registry #859
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR. I added some review remarks. Please have a look
* SPDX-License-Identifier: MIT | ||
******************************************************************************/ | ||
|
||
package org.eclipse.digitaltwin.basyx.aasdigitaltwinregistry.component.controllerAdvice; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please name the package digitaltwinregistry in a consistent way
package org.eclipse.digitaltwin.basyx.aasdigitaltwinregistry.component.controllerAdvice; | |
package org.eclipse.digitaltwin.basyx.digitaltwinregistry.component.controllerAdvice; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactored the package
path: basyx.submodelregistry/basyx.submodelregistry-service-release-log-mem/src/main/docker | ||
- name: submodel-registry-log-mongodb | ||
path: basyx.submodelregistry/basyx.submodelregistry-service-release-log-mongodb/src/main/docker | ||
- name: aas-digitaltwinregistry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- name: aas-digitaltwinregistry | |
- name: digitaltwinregistry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed
path: basyx.submodelregistry/basyx.submodelregistry-service-release-log-mem/src/main/docker | ||
- name: submodel-registry-log-mongodb | ||
path: basyx.submodelregistry/basyx.submodelregistry-service-release-log-mongodb/src/main/docker | ||
- name: aas-digitaltwinregistry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- name: aas-digitaltwinregistry | |
- name: digitaltwinregistry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remaned
@@ -0,0 +1,173 @@ | |||
/******************************************************************************* | |||
* Copyright (C) 2024 the Eclipse BaSyx Authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Copyright (C) 2024 the Eclipse BaSyx Authors | |
* Copyright (C) 2025 the Eclipse BaSyx Authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please revert cosmetic changes and version changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverted.
@ConditionalOnProperty(name = "discoveryintegration.enabled", havingValue = "true", matchIfMissing = false) | ||
public class DiscoveryIntegrationAasRegistryConfiguration { | ||
|
||
@Value("${discoveryintegration.baseUrl:#{null}}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Value("${discoveryintegration.baseUrl:#{null}}") | |
@Value("${basyx.aasregistry.feature.discoveryintegration:#{null}}") |
|
||
@Component | ||
@Slf4j | ||
@ConditionalOnProperty(name = "discoveryintegration.enabled", havingValue = "true", matchIfMissing = false) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ConditionalOnProperty(name = "discoveryintegration.enabled", havingValue = "true", matchIfMissing = false) | |
@ConditionalOnProperty(name = "basyx.aasregistry.feature.discoveryintegration.enabled", havingValue = "true", matchIfMissing = false) |
@ConditionalOnProperty(name = "discoveryintegration.enabled", havingValue = "true", matchIfMissing = false) | ||
public class DiscoveryIntegrationAasRegistryFeature implements AasRegistryStorageFeature { | ||
|
||
@Value("${discoveryintegration.baseUrl:#{null}}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Value("${discoveryintegration.baseUrl:#{null}}") | |
@Value("${basyx.aasregistry.feature.discoveryintegration:#{null}}") |
basyx.aasdigitaltwinregistry/pom.xml
Outdated
<properties> | ||
<java.version>17</java.version> | ||
<maven.compiler.source>${java.version}</maven.compiler.source> | ||
<maven.compiler.target>${java.version}</maven.compiler.target> | ||
<skipTests>false</skipTests> | ||
<maven.test.skip>false</maven.test.skip> | ||
</properties> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the properties. This is handled by the parent pom
<properties> | ||
<java.version>17</java.version> | ||
<maven.compiler.source>${java.version}</maven.compiler.source> | ||
<maven.compiler.target>${java.version}</maven.compiler.target> | ||
</properties> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove the properties
@aaronzi |
Thanks for addressing my remarks :) I meant the discovery integration feature should be added to all the AAS Registry variations, not the DTR. Sorry for the misunderstanding. |
@aaronzi Regarding the discovery integration for all AAS registry variations, could you clarify whether this refers to extending support for both the MongoDB and InMemory solutions (already been handled and tested), or if it involves creating a separate configuration within each module? Sorry for being dumb here, I am not getting this completely. |
the AAS Registry essentially has 4 components that all should have this feature:
The discovery integration feature has to be made available to all of those components via their respective POMs |
…very feature in aas registry variations
Description of Changes
This PR adds integration between AASRegistry and AASDIscovery:
/shell-description
endpoint behavior --> calling this endpoint internally calls the discover /lookup endpoint for POST callRelated Issue
BaSyx Configuration for Testing
The Digital Twin Registry supports multiple storage profiles.
Base Configuration (
application.yml
)application-MongoDB.yml
application-InMemory.yml
MongoDB-specific environment variables: