CLOUDSTACK-10012: Jetty 9.4 #2329
Conversation
client/conf/server.properties.in
Outdated
| http.port=8080 | ||
|
|
||
| # Max inactivity time in minutes for the session | ||
| session.timeout=10 |
There was a problem hiding this comment.
@marcaurele Can we bump the default to 30 minutes? I suppose 10 minutes might be too low, or maybe we can hit the sweet spot at 20mins?
There was a problem hiding this comment.
I agree that 10 minutes is very short. I would prefer 30
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✖centos6 ✔centos7 ✖debian. JID-1263 |
Signed-off-by: Rohit Yadav <[email protected]>
|
@marcaurele I've fixed the cloudian unit test failures and the session timeout to 30 (mins). |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
@marcaurele how do you start the jetty server with mvn, when I run |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1264 |
Signed-off-by: Rohit Yadav <[email protected]>
|
@marcaurele I found a workaround to use the old 9.2.x jetty-maven-plugin as the newer one did not work for me (and Travis). Please review the changes. |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✖debian. JID-1265 |
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1268 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
|
Trillian test result (tid-1658)
|
|
@blueorangutan package |
|
@rhtyd a Jenkins job has been kicked to build packages. I'll keep you posted as I make progress. |
|
Packaging result: ✔centos6 ✔centos7 ✔debian. JID-1270 |
|
@blueorangutan test |
|
@rhtyd a Trillian-Jenkins test job (centos7 mgmt + kvm-centos7) has been kicked to run smoke tests |
Signed-off-by: Marc-Aurèle Brothier <[email protected]>
|
I moved the plugin version to root pom |
|
LGTM |
| setKeystorePassword(properties.getProperty(KEYSTORE_PASSWORD)); | ||
| setWebAppLocation(properties.getProperty(WEBAPP_DIR)); | ||
| setAccessLogFile(properties.getProperty(ACCESS_LOG, "access.log")); | ||
| setSessionTimeout(Integer.valueOf(properties.getProperty(SESSION_TIMEOUT, "10"))); |
There was a problem hiding this comment.
@marcaurele you have a default value in the property file, which is 30, and here it is 10 (at line 85). Moreover, what about using the value of variable sessionTimeout as the default values? The variable already starts with the default value anyways.
There was a problem hiding this comment.
Thanks, I've synced default values to 30 now.
rafaelweingartner
left a comment
rafaelweingartner
left a comment
There was a problem hiding this comment.
LGTM from me here.
I only have a remark regarding the number of commits for such a small change. I understand that you have more than a person working here, but I believe the number of commits could be reduced.
|
@rafaelweingartner I'll squash merge them (I dislike splitting PRs in 10s+ of commits too) |
|
@rhtyd thanks! |
|
Trillian test result (tid-1659)
|
|
Tests LGTM, merged. Thanks everyone. |
|
Resurrecting an old PR - did anyone actually tested the session duration being set correctly (to something else than 30min)? It's not working in 4.11+ (whatever other value you set, the session is always set to 30min) @marcaurele @wido @rafaelweingartner /CC @rhtyd |
|
@andrijapanicsb I'll have a look and come back here with the fix |
6 participants
Upgrading to jetty 9.4 along with:
/to the context path