openanalytics / shinyproxy

ShinyProxy - Open Source Enterprise Deployment for Shiny and data science apps
https://www.shinyproxy.io
Apache License 2.0
525 stars 151 forks source link

Alligning time options shinyproxy #459

Open MohammadAliAmir opened 1 year ago

MohammadAliAmir commented 1 year ago

This is not an issue but more like an improvement.

When looking at the different options/parameters you can tweak corresponding to your needs, you can observe that the units used for time parameters are not alligned.

f.e.: heartbeat-timeout (in milliseconds), max-lifetime (in minutes). server.servlet.session.timeout (in seconds) (from https://shinyproxy.io/documentation/configuration/)

When timeout behaviour of an app changes, the developer needs to adjust the value corresponding to which time parameter he is using. It would be more efficient to allign all units of time parameters so that:

LEDfan commented 10 months ago

Hi, thank you for raising this issue.

The difference in time unit for the different properties has two reasons:

That being said, I agree that it would be better to use a single time unit, I think that using seconds would be the best option, since ShinyProxy does not really take into account milliesconds (e.g. it never check timeouts more often than once a second). The problem is that I like to keep backwards compatibility and I'm afraid changing all properties to use seconds could cause a lot of issues and confusion for users.

I can think of two work-arounds:

Add s suffix to specify the value in seconds:

For example:

heartbeat-timeout: 10000 # current behavior, i.e. milliseconds,  would still work
heartbeat-timeout: 10s # optionally users can specify the option in seconds
max-lifetime: 5 # current behavior, i.e minutes
max-lifetime: 300s

Global option

The idea is that there is a global configuration option (disabled by default), that tells ShinyProxy to read every time-based option in seconds:

use-seconds: true
heartbeat-timeout: 10
max-lifetime: 300

Both solutions still has the caveat that we cannot easily change the time unit for Spring options, so it would be good if we also check whether you can configure a time-unit for Spring.

What do you think? If anyone in the community has feedback on this it would be welcome as well!