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

Support embulk util retryhelper jetty version 94 #76

Conversation

ryannguyen360td
Copy link
Contributor

@ryannguyen360td ryannguyen360td commented Feb 28, 2024

Which issue this PR resolve for?
The new JVM uses TLS 1.3 as default, but Jetty 92 and Jetty 93 don’t support TLS 1.3.

How this PR resolve this issue?

  1. Replace embulk-util-retryhelper-jetty92 by embulk-util-retryhelper-jetty94
  2. Refactor the import path from jetty92 to jetty94
  3. Update dependencies lock:

From

org.eclipse.jetty:jetty-client:9.2.14.v20151106=compileClasspath,runtimeClasspath
org.eclipse.jetty:jetty-http:9.2.14.v20151106=compileClasspath,runtimeClasspath
org.eclipse.jetty:jetty-io:9.2.14.v20151106=compileClasspath,runtimeClasspath
org.eclipse.jetty:jetty-util:9.2.14.v20151106=compileClasspath,runtimeClasspath

To

org.eclipse.jetty:jetty-client:9.4.51.v20230217=compileClasspath,runtimeClasspath
org.eclipse.jetty:jetty-http:9.4.51.v20230217=compileClasspath,runtimeClasspath
org.eclipse.jetty:jetty-io:9.4.51.v20230217=compileClasspath,runtimeClasspath
org.eclipse.jetty:jetty-util:9.4.51.v20230217=compileClasspath,runtimeClasspath

There was the same PR, we did before:
embulk/embulk-util-retryhelper#11

Copy link
Member

@dmikurube dmikurube left a comment

Choose a reason for hiding this comment

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

LGTM.

@dmikurube dmikurube added this to the v0.7.0 milestone Mar 1, 2024
Copy link

@vietnguyen-td vietnguyen-td left a comment

Choose a reason for hiding this comment

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

LGTM

@dmikurube
Copy link
Member

Ah, one thing. Please update this part as well.

The gem distribution of this product includes JARs of the Eclipse Jetty Project (https://www.eclipse.org/jetty/) 9.2, as-is.
They are licensed under dual licenses of the Apache Software License, Version 2.0, and the Eclipse Public License 2.0.

@ryannguyen360td
Copy link
Contributor Author

Hi @dmikurube ,

I have updated NOTICE_GEM and CHANGELOG.md.

Thank you

@ryannguyen360td
Copy link
Contributor Author

Hi @dmikurube ,

Please help me to approve workflows.

Regards,

@dmikurube
Copy link
Member

Approve which workflows?

@ryannguyen360td
Copy link
Contributor Author

Approve which workflows?

Hi @dmikurube ,

Sorry for late response, as the document: https://docs.github.com/en/actions/managing-workflow-runs/approving-workflow-runs-from-public-forks

When an outside contributor submits a pull request to a public repository, a maintainer with write access may need to approve any workflow runs.

As, I checked the workflow has been run:
image

So, you can ignore my previous comment.

@ryannguyen360td
Copy link
Contributor Author

Hi @dmikurube ,

Please help me merge & release this PR.

Thank you,

@dmikurube
Copy link
Member

Other pull requests are merged. It's time to go. Thanks for working on it!

@dmikurube dmikurube merged commit f90a962 into embulk:master Jul 26, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants