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

Prepare plugin for Elasticsearch 7.17.20 #41

Open
wants to merge 5 commits into
base: elasticsearch-7
Choose a base branch
from

Conversation

rprodan
Copy link

@rprodan rprodan commented Feb 14, 2024

@vhyza I bumped dependency versions for making it compatible with Elasticsearch 7.17.18.

@rprodan rprodan mentioned this pull request Feb 14, 2024
@rprodan
Copy link
Author

rprodan commented Feb 29, 2024

@vhyza when do you expect to be able to prepare a release? I'm happy to help if anything else is needed. Thank you.

@vhyza
Copy link
Owner

vhyza commented Mar 1, 2024

Hi @rprodan,

to be honest I really don't know. For almost a year I didn't find the time/energy to make a new release so it accumulated quite a lot and now it is quite discouraging to even begin.

So I'd like to address first PR from @crabhi which could help with automatic builds and another PR from @vit-zikmund to simplify the dependencies a little bit.

The problem is that I need to try this on both master and elasticsearch-7 branches as Elastic is still releasing 7.x versions. Also, I changed a computer and now I see that the tests are not working (as mentioned in the PRs above) which is weird as they worked on the old computer 🤷‍♂️.

I do not think I'll be able to address this PR in the following days or weeks. If you need the packaged .zip I suppose I could upload it here when I'm able to run the tests. But I'm on arm64 (as a non-java programmer I do not know if it would work or not)...

I'm sorry I do not have better news...

@rprodan
Copy link
Author

rprodan commented Mar 20, 2024

@vhyza thank you for your answer. I completely understand you. If you need any help with this work please let me know. Regards, and keep good energy/spirit. 👍

@rprodan rprodan changed the title Prepare plugin for Elasticsearch 7.17.18 Prepare plugin for Elasticsearch 7.17.19 Mar 28, 2024
@rprodan
Copy link
Author

rprodan commented Mar 28, 2024

@vhyza the issue with tests were the following dependencies

<dependency>
      <groupId>org.apache.logging.log4j</groupId>
      <artifactId>log4j-core</artifactId>
      <version>[2.16.0,)</version>
      <scope>test</scope>
</dependency>
<dependency>
      <groupId>org.apache.logging.log4j</groupId>
      <artifactId>log4j-api</artifactId>
      <version>[2.16.0,)</version>
      <scope>test</scope>
 </dependency>

You specified [2.16.0,) and this means Maven is fetching last version available from Maven Central.

Artefact log4j-api-3.0.0-beta2 was fetched which requires classes compiled with Java 17 at minimum.

The version has been set to latest 2.x.x and now the tests are working as expected.

@rprodan
Copy link
Author

rprodan commented Mar 28, 2024

@vhyza is it safe to build a package from my local environment and use .zip artefact where needed? In case you don't have time to perform official release.

@vit-zikmund
Copy link

Hi @rprodan just cruising by, but FWIW I managed to get rid of the test failures also by removing these dependencies for good. It didn't seem to break anything. You can look at my PR for how to build the stuff (using temurin 17 docker image). You should be able to use the zip then. At least I was, until I hit the log4j's security problem, that likely doesn't exist yet at java 8.

@rprodan rprodan changed the title Prepare plugin for Elasticsearch 7.17.19 Prepare plugin for Elasticsearch 7.17.20 Apr 10, 2024
@rprodan
Copy link
Author

rprodan commented Apr 10, 2024

Bumped version of ES client dependency to 7.17.20 due to issue with JDK 22 in 7.17.19 (see https://www.elastic.co/guide/en/elasticsearch/reference/7.17/release-notes-7.17.19.html)

@vit-zikmund thank you for your comment. I built the package (elasticsearch-analysis-lemmagen-7.17.20-plugin.zip) myself with maven clean install command

@vhyza if you have time you can easily merge this :)

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.

3 participants