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

Make a variable for backend timeout #53

Closed
denissimonovski opened this issue Jun 17, 2021 · 9 comments · Fixed by #70
Closed

Make a variable for backend timeout #53

denissimonovski opened this issue Jun 17, 2021 · 9 comments · Fixed by #70
Labels
enhancement New feature or request P2 high priority issues triaged Scoped and ready for work

Comments

@denissimonovski
Copy link
Contributor

Can we refactor this hardcoded value with a variable?

timeout_sec = 10

@morgante
Copy link
Contributor

Should we potentially just use the same timeout as the health_check?

@denissimonovski
Copy link
Contributor Author

I think that would be way better than this. It would be a great start, at least.

@morgante morgante added enhancement New feature or request P2 high priority issues triaged Scoped and ready for work labels Jun 17, 2021
@morgante
Copy link
Contributor

Sounds good, I've put it into our backlog. If you want to open a pull request, we'd be happy to review and it will likely land sooner.

@denissimonovski
Copy link
Contributor Author

That's great. I'll take a look into this next week.

@denissimonovski
Copy link
Contributor Author

@morgante I found this in the GCP docs:

For internal TCP/UDP load balancers and network load balancers, you can set the value of the backend service timeout using gcloud or the API, but the value is ignored. Backend service timeout has no meaning for these pass-through load balancers.
Source: https://cloud.google.com/load-balancing/docs/backend-service#timeout-setting

So whatever we put in the line I opened my issue for, doesn't make any difference. Since this is already an optional variable is it better to remove that line altogether and keep it clean? I can do it if you approve.

@morgante
Copy link
Contributor

@denissimonovski Yes, go ahead and remove it. Please also leave a comment linking to this issue so we remember not to add it back.

@denissimonovski
Copy link
Contributor Author

denissimonovski commented Aug 12, 2021

@morgante can you take a look at the open PR below?

@denissimonovski
Copy link
Contributor Author

@morgante how about now?

@denissimonovski
Copy link
Contributor Author

@morgante now?

morgante pushed a commit that referenced this issue Aug 13, 2021
* Remove timeout_sec from google_compute_region_backend_service because it's ignored ([#53](#53))

* Add comment to not re-add timeout_sec attribute

* Add comment to not re-add timeout_sec attribute

* terraform fmt
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request P2 high priority issues triaged Scoped and ready for work
Projects
None yet
2 participants