Skip to content

fix: resolve gf-connect is not published#645

Merged
SemyonSinchenko merged 11 commits into
graphframes:masterfrom
SemyonSinchenko:639-fix-connect-publishing
Jul 17, 2025
Merged

fix: resolve gf-connect is not published#645
SemyonSinchenko merged 11 commits into
graphframes:masterfrom
SemyonSinchenko:639-fix-connect-publishing

Conversation

@SemyonSinchenko

@SemyonSinchenko SemyonSinchenko commented Jul 17, 2025

Copy link
Copy Markdown
Collaborator

What changes were proposed in this pull request?

Refactoring according the comment

Why are the changes needed?

Close #639

@SemyonSinchenko SemyonSinchenko changed the title 639 fix connect publishing fix: resolve gf-connect is not published Jul 17, 2025
@SemyonSinchenko SemyonSinchenko self-assigned this Jul 17, 2025
@SemyonSinchenko

SemyonSinchenko commented Jul 17, 2025

Copy link
Copy Markdown
Collaborator Author

Thanks @Kimahriman for the suggestion! I tried publishLocal and it works as expected!

Kimahriman
Kimahriman reviewed Jul 17, 2025
View reviewed changes
Comment thread .github/workflows/scala-ci.yml Outdated
Kimahriman
Kimahriman reviewed Jul 17, 2025
View reviewed changes
Comment thread .github/workflows/scala-ci.yml Outdated
@Kimahriman

Kimahriman commented Jul 17, 2025

Copy link
Copy Markdown
Contributor

IMO it would be cleaner to just have core/ and connect/ folders, instead of having both prefixed with graphframes-, but up to you

SemyonSinchenko and others added 2 commits July 17, 2025 18:05
@SemyonSinchenko @Kimahriman
Update .github/workflows/scala-ci.yml
07fa477 
Co-authored-by: Adam Binford <adamq43@gmail.com>
@SemyonSinchenko @Kimahriman
Update .github/workflows/scala-ci.yml
2edf3b9 
Co-authored-by: Adam Binford <adamq43@gmail.com>
Kimahriman
Kimahriman reviewed Jul 17, 2025
View reviewed changes
Comment thread .github/workflows/scala-ci.yml Outdated
@Kimahriman

Kimahriman commented Jul 17, 2025

Copy link
Copy Markdown
Contributor

One last other thing I noticed. The Spark 3 build is built with Java 11, but Spark 3 does support Java 8 so the released artifacts should be built with Java 8

@SemyonSinchenko @Kimahriman
Update .github/workflows/scala-ci.yml
6fff061 
Co-authored-by: Adam Binford <adamq43@gmail.com>
@SemyonSinchenko

SemyonSinchenko commented Jul 17, 2025

Copy link
Copy Markdown
Collaborator Author

And again thanks a lot @Kimahriman ! Your experience is helping a lot to GraphFarmes to become better!!

@SemyonSinchenko

SemyonSinchenko commented Jul 17, 2025

Copy link
Copy Markdown
Collaborator Author

I will update

rjurney
rjurney approved these changes Jul 17, 2025
View reviewed changes

@rjurney rjurney left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lgtm, looks like core got left out previously?

@rjurney

rjurney commented Jul 17, 2025

Copy link
Copy Markdown
Collaborator

Oh wait, CI failed...

Kimahriman
Kimahriman reviewed Jul 17, 2025
View reviewed changes
Comment thread .github/workflows/scala-ci.yml Outdated
@SemyonSinchenko @Kimahriman
Update .github/workflows/scala-ci.yml
391b596 
Co-authored-by: Adam Binford <adamq43@gmail.com>
@SemyonSinchenko

SemyonSinchenko commented Jul 17, 2025

Copy link
Copy Markdown
Collaborator Author

lgtm, looks like core got left out previously?

@Kimahriman resolved the puzzle (see comments in the issue). Mostly it was my fault, I was needed to refactor it to the right multi-project sbt layout when I introduced connect.

@SemyonSinchenko SemyonSinchenko merged commit 7c5ae09 into graphframes:master Jul 17, 2025
5 checks passed
@SemyonSinchenko SemyonSinchenko deleted the 639-fix-connect-publishing branch July 17, 2025 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rjurney rjurney rjurney approved these changes
@james-willis james-willis Awaiting requested review from james-willis
+1 more reviewer
@Kimahriman Kimahriman Kimahriman left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

@SemyonSinchenko SemyonSinchenko

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: graphframes-connect is not published

3 participants

@SemyonSinchenko @Kimahriman @rjurney