Skip to content

allow to set che ws agent inacive timeout in helm#10705

Merged
riuvshin merged 5 commits intomasterfrom
riuvshin-patch-1
Aug 9, 2018
Merged

allow to set che ws agent inacive timeout in helm#10705
riuvshin merged 5 commits intomasterfrom
riuvshin-patch-1

Conversation

@riuvshin
Copy link
Copy Markdown
Contributor

@riuvshin riuvshin commented Aug 8, 2018

allow to set che ws agent inacive timeout in helm

@riuvshin riuvshin self-assigned this Aug 8, 2018
@riuvshin riuvshin requested a review from a user August 8, 2018 15:57
Copy link
Copy Markdown
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

Please consider using new property instead of deprecated one

CHE_WORKSPACE_HTTPS__PROXY: {{ .Values.cheWorkspaceHttpsProxy }}
CHE_WORKSPACE_NO__PROXY: {{ .Values.cheWorkspaceNoProxy }}
CHE_WORKSPACE_AGENT_DEV_INACTIVE__STOP__TIMEOUT__MS: {{ .Values.global.wsAgentInactiveStopTimeout }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove extra line

gitHubClientSecret: ""
pvcClaim: "1Gi"
cheWorkspacesNamespace: ""
wsAgentInactiveStopTimeout: "0"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would say that it would be better to use default value here that is "-1"

CHE_WORKSPACE_HTTP__PROXY: {{ .Values.cheWorkspaceHttpProxy }}
CHE_WORKSPACE_HTTPS__PROXY: {{ .Values.cheWorkspaceHttpsProxy }}
CHE_WORKSPACE_NO__PROXY: {{ .Values.cheWorkspaceNoProxy }}
CHE_WORKSPACE_AGENT_DEV_INACTIVE__STOP__TIMEOUT__MS: {{ .Values.global.wsAgentInactiveStopTimeout }}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As far as I know, it is deprecated property, it is done in https://github.com/eclipse/che/blob/44588aa775e77f629eb0a0052ac20d949ab8c7c3/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che_aliases.properties#L36 Consider using new one which is che.limits.workspace.idle.timeout

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah! nice catch, btw I thought 0 is a default value.

Copy link
Copy Markdown
Member

@sleshchenko sleshchenko left a comment

Choose a reason for hiding this comment

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

Now LGTM. Thanks for adding such an ability 👍 👍 👍

@riuvshin riuvshin merged commit 5d26caf into master Aug 9, 2018
@riuvshin riuvshin deleted the riuvshin-patch-1 branch August 9, 2018 08:37
@benoitf benoitf added this to the 6.10.0 milestone Aug 9, 2018
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.

3 participants