Skip to content
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

fix: project support #58

Merged
merged 2 commits into from
Jul 12, 2023
Merged

fix: project support #58

merged 2 commits into from
Jul 12, 2023

Conversation

ClemDoum
Copy link
Collaborator

@ClemDoum ClemDoum commented Jul 12, 2023

PR description

While multiple project had been properly added on the neo4j side in #37, the extension was still setup to use a single index on the ES side and hence was not properly configuredfor a server usage. This PR adds a proper support of multiproject on the ES side.

Changes

datashare-neo4j-extension/src

Changed

  • Forward the project index alongside the project database in importNamedEntities, importDocuments and exportNeo4jCSVs

datashare-neo4j-extension/neo4j-app

Remove

  • Removed the AppConfig.neo4j_project attribute

Changed

  • Removed the ESClient.project_index attribute, the ES client now takes the index as method arguments
  • Updated the POST /neo4j-csvs, POST /documents, POST /named-entities and underlying neo4j.core.import functions to take the ES index as input

@ClemDoum ClemDoum self-assigned this Jul 12, 2023
@ClemDoum ClemDoum added elasticsearch bug Something isn't working labels Jul 12, 2023
@ClemDoum ClemDoum marked this pull request as ready for review July 12, 2023 08:49
@ClemDoum ClemDoum requested a review from a team as a code owner July 12, 2023 08:49
Copy link
Member

@pirhoo pirhoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@ClemDoum
Copy link
Collaborator Author

Thanks for the review !

@ClemDoum ClemDoum merged commit aafa300 into main Jul 12, 2023
3 checks passed
@ClemDoum ClemDoum deleted the fix/project branch July 12, 2023 08:56
Copy link
Contributor

@caro3801 caro3801 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great !

@@ -366,7 +366,7 @@ protected org.icij.datashare.Objects.IncrementalImportResponse importDocuments(
checkExtensionProject(projectId);
checkNeo4jAppStarted();
String db = neo4jProjectDatabase(projectId);
return client.importDocuments(db, request);
return client.importDocuments(db, projectId, request);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for info, you change here your semantic from index to projectId. As far as I saw, there is no mention of projectId elsewhere

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working elasticsearch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants