Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[TOQo1n9M] apoc-hadoop dependency is conflicting #3450

Merged
merged 1 commit into from
Feb 16, 2023

Conversation

vga91
Copy link
Collaborator

@vga91 vga91 commented Feb 14, 2023

  • Excluded packages from hadoop extra-deps
  • added addExtraDependencies() in TestContainerUtil
  • added tests with core/full jar together with all extra-dependencies

@vga91 vga91 added the 4.4 label Feb 14, 2023
@vga91 vga91 force-pushed the hadoop-dependency-conflict branch from 3340137 to 680a8e2 Compare February 15, 2023 08:04
@Lojjs Lojjs self-assigned this Feb 15, 2023
Copy link
Contributor

@Lojjs Lojjs left a comment

Choose a reason for hiding this comment

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

Nice, I really like the refactorings you have done too! Just one small question.

Comment on lines +33 to +34
return Paths.get( basePath.concat(project) )
.toFile();
Copy link
Contributor

Choose a reason for hiding this comment

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

Comparing with the old code, there was a slash in the concat method: .concat("/full") now this will be .concat(full). Does that matter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I forgot to mention it.

How it was before wasn't totally right,
because basePath is for example /My/Path/neo4j-apoc-procedures/.

By doing basePath.concat("/full") becomes /My/Path/neo4j-apoc-procedures//full (with a double slash before full).

Then Paths.get(...) removes the double slash indeed,
but I guess it's better to remove it beforehand.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok that makes sense, thanks for explaining

@vga91 vga91 merged commit 1af1f57 into 4.4 Feb 16, 2023
@vga91 vga91 deleted the hadoop-dependency-conflict branch February 16, 2023 13:31
vga91 added a commit to neo4j/apoc that referenced this pull request Apr 3, 2023
…4j-contrib/neo4j-apoc-procedures#3450) (#347)

* [TOQo1n9M] Core part - apoc-hadoop dependency is conflicting 5.x (neo4j-contrib/neo4j-apoc-procedures#3450)

* [TOQo1n9M] changed getProjectPath() to isExtendedProject()
vga91 added a commit to neo4j/apoc that referenced this pull request Apr 12, 2023
…4j-contrib/neo4j-apoc-procedures#3450) (#347)

* [TOQo1n9M] Core part - apoc-hadoop dependency is conflicting 5.x (neo4j-contrib/neo4j-apoc-procedures#3450)

* [TOQo1n9M] changed getProjectPath() to isExtendedProject()
vga91 added a commit that referenced this pull request Apr 18, 2023
* [TOQo1n9M] apoc-hadoop dependency is conflicting (#3450)

* [TOQo1n9M] changed for 5.x version

* [TOQo1n9M] update .gitmodules branch

* [TOQo1n9M] removed imports

* [TOQo1n9M] fixed compile error
vga91 added a commit that referenced this pull request Apr 21, 2023
* [TOQo1n9M] apoc-hadoop dependency is conflicting (#3450)

* [TOQo1n9M] changed for 5.x version

* [TOQo1n9M] update .gitmodules branch

* [TOQo1n9M] removed imports

* [TOQo1n9M] fixed compile error
vga91 added a commit that referenced this pull request Apr 21, 2023
* [TOQo1n9M] apoc-hadoop dependency is conflicting (#3450)

* [TOQo1n9M] changed for 5.x version

* [TOQo1n9M] update .gitmodules branch

* [TOQo1n9M] removed imports

* [TOQo1n9M] fixed compile error
vga91 added a commit that referenced this pull request Apr 28, 2023
* [TOQo1n9M] apoc-hadoop dependency is conflicting (#3450)

* [TOQo1n9M] changed for 5.x version

* [TOQo1n9M] update .gitmodules branch

* [TOQo1n9M] removed imports

* [TOQo1n9M] fixed compile error
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants