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

[ci] publish snapshots with every commit to main #16

Merged
merged 2 commits into from Jan 31, 2024

Conversation

Shastick
Copy link
Collaborator

@Shastick Shastick commented Jul 7, 2023

Adds a workflow for publishing a snapshot to sonatype with each commit to main.

@Shastick Shastick force-pushed the snapshot-releases branch 3 times, most recently from c1be77a to f207bfb Compare July 7, 2023 21:21
@langchain4j
Copy link
Owner

@Shastick could you please provide a bit of details, I am not sure I understand how it works and why we need this. Thanks a lot!

@Shastick Shastick marked this pull request as draft July 21, 2023 15:21
@Shastick
Copy link
Collaborator Author

This was meant as a draft to try out publishing to sonatype via github actions. I'll reopen when I had some time to explore this

@Shastick Shastick changed the title Draft workflow for publishing snapshots with every commit to main [draft] Draft workflow for publishing snapshots with every commit to main Jul 21, 2023
@nooone
Copy link

nooone commented Sep 27, 2023

This would actually be pretty useful, especially while this is still under active development. Would be nice to get bugfixes and new features faster than once every few weeks with a new release.

@Shastick
Copy link
Collaborator Author

Shastick commented Oct 6, 2023

@langchain4j should I take a look at this again?

I’ll likely need some credentials to push to Sonatype:

You could either grant me access (I have a sonatype account that I could give you the name off), or add your credentials as secrets somewhere in the Github actions settings.

@langchain4j
Copy link
Owner

@Shastick where can/should we host snapshots? By Sonatype you mean the same repo where we host released artefacts?

@Shastick
Copy link
Collaborator Author

Shastick commented Oct 6, 2023

@Shastick where can/should we host snapshots? By Sonatype you mean the same repo where we host released artefacts?

Yes, at sonatype: if I remember correctly the process is roughly the same as for a normal release, except that:

  • you'll push to the snapshot repository (plugins usually do this automatically based on the version)
  • there is no need to manually promote the artifact (again, if I remember correctly)

@langchain4j
Copy link
Owner

Hi @Shastick, sorry for the delay. I have added OSSRH_USERNAME and OSSRH_PASSWORD repository secrets, I guess this should be enough to publish snapshots?

@Shastick Shastick force-pushed the snapshot-releases branch 10 times, most recently from c9d20ec to 06b90fc Compare January 19, 2024 09:40
@Shastick
Copy link
Collaborator Author

Shastick commented Jan 19, 2024

Hi @Shastick, sorry for the delay. I have added OSSRH_USERNAME and OSSRH_PASSWORD repository secrets, I guess this should be enough to publish snapshots?

I'm giving this a try and seem to get 401's from sonatype: are the credentials still up to date?

Ie, we get:

Failed to execute goal org.sonatype.plugins:nexus-staging-maven-plugin:1.6.13:deploy (injected-nexus-deploy) on project ***-document-parser-apache-poi: Failed to deploy artifacts: Could not transfer artifact dev.***:***-core:jar:javadoc:0.26.0-20240119.094028-2 from/to ossrh (https://s01.oss.sonatype.org/content/repositories/snapshots): authentication failed for https://s01.oss.sonatype.org/content/repositories/snapshots/dev/***/***-core/0.26.0-SNAPSHOT/***-core-0.26.0-20240119.094028-2-javadoc.jar, status: 401 Unauthorized -> [Help 1]

For example on this run

@langchain4j could you try running a snapshot build locally (I assume it's a mvn deploy or mvn --batch-mode deploy on the master branch without changing any version) with the credentials set in MAVEN_USERNAME and MAVEN_PASSWORD and report if it works or not?

@langchain4j
Copy link
Owner

Hi @Shastick sorry for delay.

I have set MAVEN_USERNAME and MAVEN_PASSWORD env variables locally and run mvn clean deploy -DskipTests on current master, it works fine: https://s01.oss.sonatype.org/content/repositories/snapshots/dev/langchain4j/langchain4j-core/0.26.0-SNAPSHOT/

I am not sure the need for MAVEN_USERNAME and MAVEN_PASSWORD as I have user/password configured in .m2/settings.xml.

I have updated OSSRH_PASSWORD and OSSRH_TOKEN with same values as in my .m2/settings.xml just in case.

@langchain4j
Copy link
Owner

Maybe try using OSSRH_PASSWORD and OSSRH_TOKEN names for env variables? This worked fine for me: https://github.com/ai-for-java/openai4j/blob/main/.github/workflows/release.yml

@Shastick
Copy link
Collaborator Author

Shastick commented Jan 24, 2024

@langchain4j Looks like we make it one step further, but now:

Deployment failed: repository element was not specified in the POM inside distributionManagement element or in -DaltDeploymentRepository=id::layout::url parameter -> [Help 1]

I'll try to check later, but possibly you have something in your settings about distributionManagement that we could add to the aggregator pom?

@langchain4j
Copy link
Owner

@Shastick I am getting the same error locally. I guess it tries to deploy aggregator pom which does not contain all mandatory information like scm links, etc. But we do not publish anyway, so we could exclude this from the build. Other modules are published ok as far as I see, right?

BTW could you please remind why we went with this parent/aggregator separation?

@Shastick
Copy link
Collaborator Author

Looks like snapshot publishing was successful, I'll update the action so it only runs on master and this should be ready to go.

@Shastick Shastick marked this pull request as ready for review January 24, 2024 16:02
@Shastick Shastick changed the title [draft] Draft workflow for publishing snapshots with every commit to main [ci] publish snapshots with every commit to main Jan 24, 2024
langchain4j
langchain4j previously approved these changes Jan 24, 2024
OSSRH_USERNAME: ${{ secrets.OSSRH_USERNAME }}
OSSRH_PASSWORD: ${{ secrets.OSSRH_PASSWORD }}
# These don't work with java 8.
# Consider have something separate with more recent versions for the modules that don't support older versions?
Copy link
Owner

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed that would make sense.

What is the approach for releases? Is everything released from the older version of Java that is possible?

If you have a script or a bunch of options that are used for release they could be useful here, otherwise I can cobble something together from the existing workflow files.

Copy link
Owner

Choose a reason for hiding this comment

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

What is the approach for releases? Is everything released from the older version of Java that is possible?

Yes,

  • we release with java 8 everything except langchain4j-opensearch, langchain4j-neo4j and langchain4j-code-execution-engine-graalvm-polyglot
  • then langchain4j-opensearch with 11
  • then langchain4j-neo4j and langchain4j-code-execution-engine-graalvm-polyglot with 17

I do not have any script unfortunately.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll have a look, I think that should be feasible in some way

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

One side question though: is it important that all snapshots for a release from master share the same version?

Currently it seems based on time: I assume we could have something based on the commit hash if required.

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, I guess users should be able to use X.Y.Z-SNAPSHOT version and maven should provide the latest snapshot, or?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok, makes sense. I'll look into that once I got the rest to work

pom.xml Outdated Show resolved Hide resolved
@langchain4j
Copy link
Owner

@Shastick thanks a lot! What was the problem BTW? wrong env variable names?

@Shastick
Copy link
Collaborator Author

@Shastick thanks a lot! What was the problem BTW? wrong env variable names?

Yes, it seems I got confused with the environment variables

@Shastick
Copy link
Collaborator Author

@langchain4j did we maybe overthink things a bit? Given the maven.compiler.target options in the pom files, I believe that building and releasing everything from a recent version of java should still produce jars that can be consumed by an older JVM where possible?

@Shastick
Copy link
Collaborator Author

Shastick commented Jan 25, 2024

Release works (see this run for example).

I assume we can trust the backward compatibility (all modules specify the minimal java version to target), so we don't need to build on multiple JDKs (and we can always revisit if required)

Also, it seems that the version is as expected: ie, here is the metadata for langchain4j-neo4j:0.26.0-SNAPSHOT. Multiple releases of the same snapshot version can be done.

All modules can be seen here: https://s01.oss.sonatype.org/content/repositories/snapshots/dev/langchain4j/

@Shastick Shastick force-pushed the snapshot-releases branch 3 times, most recently from 8aa94ce to 8f158cf Compare January 25, 2024 22:24
@langchain4j
Copy link
Owner

@Shastick thank you so much and sorry for the late reply. Yeah seems that I indeed overthinked it and we can have a single build. Awesome!

Copy link
Owner

@langchain4j langchain4j left a comment

Choose a reason for hiding this comment

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

@Shastick great job, thank you!

@langchain4j langchain4j merged commit 4cef820 into main Jan 31, 2024
6 checks passed
@langchain4j langchain4j deleted the snapshot-releases branch January 31, 2024 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants