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

What is the jetty.deploy.scanInterval default? module, ini, code, and documentation do not agree. #11424

Closed
razisam opened this issue Feb 20, 2024 · 3 comments · Fixed by #11437
Assignees
Labels

Comments

@razisam
Copy link

razisam commented Feb 20, 2024

Jetty version(s)
Jetty 12.0.6

Jetty Environment
ee9

Java version/vendor (use: java -version)
jdk21

OS type/version
linux

Description
i was sure, the suggestions in ee9-deploy.mod/.ini are telling us, what the default-value is and if we want it another way, we can change it in the ini file:

...
## Monitored directory scan period (seconds)
# jetty.deploy.scanInterval=1
...

but looking into the code, the default of this scanInterval is "0":
image

is this a bug or some feature, which i do not find any default value for in documentation?
maybe it's also just missing "default=1" in /etc/jetty-ee9-deploy.xml:

<Set name="scanInterval" property="jetty.deploy.scanInterval"/>
-->
<Set name="scanInterval" property="jetty.deploy.scanInterval" default="1"/>

@razisam razisam added the Bug For general bugs on Jetty side label Feb 20, 2024
@joakime
Copy link
Contributor

joakime commented Feb 20, 2024

The Jetty XML syntax ...

<Set name="foo" property="propname"/>

means that the setter .setFoo(...) will be called if the property propname is set.
If the property propname is unset, then the named setter is not even called.

This means the defaults in code win if you don't set a the property.

This is expected and intentional behavior, not a bug in the XML that needs addressing.
The commented out example property in the module and ini file could be updated, but ultimately those are just examples, not default values.

@joakime joakime added Question and removed Bug For general bugs on Jetty side labels Feb 20, 2024
@razisam
Copy link
Author

razisam commented Feb 20, 2024

This is expected and intentional behavior, not a bug in the XML that needs addressing. The commented out example property in the module and ini file could be updated, but ultimately those are just examples, not default values.

debugging/klicking through jetty code should not be the source of information.
but maybe i missed the "default-value" documentation somewhere. it was "1" in the previous versions and was changed in jetty 12. so if you upgrade von jetty11 to jetty12, there is no chance to see this change, only by noticing the "new behaviour", so please change the "ini-template" at least. it might be an example only, but i'ms sure it's missleading because many people think, it's also the default value.

Also look here:

image

@joakime
Copy link
Contributor

joakime commented Feb 20, 2024

Now documentation, that's an important thing to address.
Also, WHY is it defaulting to 0? (need to dig into the history for that)

But still, the XML doesn't need changing to address either of those concerns.

@joakime joakime changed the title jetty.deploy.scanInterval = 1 What is the jetty.deploy.scanInterval default? module, ini, code, and documentation do not agree. Feb 20, 2024
@joakime joakime moved this to 🏗 In progress in Jetty 12.0.7 - FROZEN Feb 22, 2024
@joakime joakime self-assigned this Feb 22, 2024
joakime added a commit that referenced this issue Feb 22, 2024
…to `1`

+ Leaving embedded default alone (at `0`)
joakime added a commit that referenced this issue Feb 22, 2024
joakime added a commit that referenced this issue Feb 22, 2024
…#11437)

+ Update documentation and defaults in mod files
@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 12.0.7 - FROZEN Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants