-
Notifications
You must be signed in to change notification settings - Fork 64
[1694] Customize Elasticsearch indices to work with SysML #1834
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?
Conversation
c9041b5 to
968d591
Compare
| - https://github.com/eclipse-syson/syson/issues/1694[#1694] [syson] Customize Elasticsearch indices to work with SysML. | ||
| SysON now uses its own indexing logic instead of the default one provided by Sirius Web. | ||
| This allows to limit the size of the indices, and ensure information stored in the indices are useful to perform cross-project search. | ||
| You can find more information on how to setup Elasticsearch, how elements are mapped to index documents, and how to query them in the documentation. |
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.
I didn't add a link to the documentation because the page doesn't exist yet (it is added by this PR)
| @Override | ||
| public boolean canHandle(IEditingContext editingContext, Object object) { | ||
| // Only produce SysON index entries for non-studio projects. | ||
| return !this.studioCapableEditingContextPredicate.test(editingContext.getId()); |
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.
I used the same condition here as we did in #944 to discriminate SysON projects from Studio projects.
| } | ||
|
|
||
| private void clearIndex(IEditingContext editingContext) { | ||
| this.indexDeletionService.deleteIndex(editingContext.getId()); |
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.
In some cases (particularly when a new project is created), this line may throw an exception indicating that the index does not exist. This exception doesn't crash the application, it just prevents an update of the index, which is not critical (it will be updated later when the semantic data gets updated).
This issue is linked to eclipse-sirius/sirius-web#6044, and should be fixed in Sirius Web, not SysON.
| registry.add("spring.elasticsearch.uris", ELASTICSEARCH_CONTAINER::getHttpHostAddress); | ||
| registry.add("spring.elasticsearch.username", () -> "elastic"); | ||
| registry.add("spring.elasticsearch.password", () -> ElasticsearchContainer.ELASTICSEARCH_DEFAULT_PASSWORD); |
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.
I used the same approach we use in Sirius Web: Elasticsearch is enabled for all the tests, it should be possible to check the indices anytime, even in existing test cases.
If this slows significantly the build we can think of an alternative solution.
doc/shapes/2026.6.1/customize_elasticsearch_index_to_work_with_sysml.adoc
Show resolved
Hide resolved
|
Note that this PR doesn't add Elasticsearch in the docker compose file, I don't have the expertise to do this, but it is currently a work in progress in a downstream project. |
Bug: #1694 Signed-off-by: Gwendal Daniel <gwendal.daniel@obeosoft.com> # Conflicts: # CHANGELOG.adoc # doc/content/modules/user-manual/pages/release-notes/2026.1.0.adoc
968d591 to
234eb4d
Compare
| ownedSpecializationNestedIndexEntries, ownedElementNestedIndexEntries)); | ||
| } | ||
|
|
||
| @Override |
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 put this method in the beginning of the class
| } | ||
|
|
||
| @Override | ||
| public Optional<IIndexEntry> doSwitch(EObject eObject) { |
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 put this method in the beginning of the class
| @JsonProperty(IIndexEntry.LABEL_FIELD) | ||
| String label(); | ||
|
|
||
|
|
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.
useless empty line
| * | ||
| * @author gdaniel | ||
| */ | ||
| public record NamespaceIndexEntry( |
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.
Could you please add javadoc for each parameter of the record?
| * | ||
| * @author gdaniel | ||
| */ | ||
| public record NestedElementIndexEntry( |
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.
Could you please add javadoc for each parameter of the record?
| } | ||
|
|
||
| @Override | ||
| public Optional<INestedIndexEntry> doSwitch(EObject eObject) { |
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 put this method in the beginning of the class
| * | ||
| * @author gdaniel | ||
| */ | ||
| public record NestedSpecializationIndexEntry( |
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.
Could you please add javadoc for each parameter of the record? And add some details about the role of this record
| * | ||
| * @author gdaniel | ||
| */ | ||
| public record TypeIndexEntry( |
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.
Could you please add javadoc for each parameter of the record? And add some details about the purpose of this record.
| import co.elastic.clients.util.NamedValue; | ||
|
|
||
| /** | ||
| * Creates indices to store SysML models. |
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.
Could you please add some detail about the purpose of this class?
| * @author gdaniel | ||
| */ | ||
| @JsonTypeInfo(use = JsonTypeInfo.Id.CLASS, include = JsonTypeInfo.As.PROPERTY, property = "@nestedIndexEntryType") | ||
| public interface INestedIndexEntry { |
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.
Could you please add some detail about the purpose of this interface?
|
First review, I need to do a second one. |
Bug: #1694
PLEASE READ ALL ITEMS AND CHECK ONLY RELEVANT CHECKBOXES BELOW
Auto review
Project management
priority:andpr:labels been added to the pull request? (In case of doubt, start with the labelspriority: lowandpr: to review later)area:,type:)Changelog and release notes
CHANGELOG.adoc+doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adocbeen updated to reference the relevant issues?CHANGELOG.adoc?CHANGELOG.adoc?doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?Key highlightssection indoc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?Documentation
Tests