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

Cbayer jdk17 spring 3.0.2 #588

Merged
merged 17 commits into from
Feb 10, 2023
Merged

Cbayer jdk17 spring 3.0.2 #588

merged 17 commits into from
Feb 10, 2023

Conversation

chrbayer84
Copy link
Contributor

@chrbayer84 chrbayer84 commented Jan 23, 2023

This resolves #151 #584 #584 #586 #587 #583 #593

@chrbayer84
Copy link
Contributor Author

tests still failing though. App context doesn't come up.

build.gradle Outdated
@@ -5,14 +5,14 @@ buildscript {
jcenter()
}
dependencies {
classpath("org.springframework.boot:spring-boot-gradle-plugin:2.5.6")
classpath("org.springframework.boot:spring-boot-gradle-plugin:3.0.2")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not add this to the top-level gradle.properties instead?

exclude group: 'org.apache.logging.log4j', module: 'log4j-to-slf4j'
exclude group: 'org.apache.logging.log4j', module: 'log4j-api'
}
compile group:"org.springframework.boot", name:"spring-boot-starter-web", version:"2.7.5"
compile group:"org.springframework.boot", name:"spring-boot-starter-jetty", version:"2.7.5"
implementation group:"org.springframework.boot", name:"spring-boot-starter-web", version:"3.0.2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't need a version on dependencies when using the spring boot plugin.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

unfortunately it is needed here.. I think it works for project dependencies but not for buildscript dependencies.

ncsaLog.setAppend( true );
ncsaLog.setLogTimeZone( "GMT" );
ncsaLog.setRetainDays( jettyRequestLogRetentionDays );
// NCSARequestLog ncsaLog = new NCSARequestLog( jettyLogfilePath );
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing request log?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. The class was removed from Jetty. Not sure how to get this functionality back.


}

test {
maxHeapSize = "2g"
jvmArgs = [
'--add-opens', 'java.base/java.util=ALL-UNNAMED',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If that's removed, one of the test cases around CarbonJEvent logs will fail.

@slodha slodha requested a review from peter-z-xu February 2, 2023 14:31
@chrbayer84 chrbayer84 merged commit 2e98c23 into master Feb 10, 2023
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.

2 participants