Skip to content
This repository has been archived by the owner on Oct 7, 2024. It is now read-only.

Make kafka startup timeout configurable #58

Merged
merged 3 commits into from
Sep 9, 2016
Merged

Make kafka startup timeout configurable #58

merged 3 commits into from
Sep 9, 2016

Conversation

antban
Copy link
Contributor

@antban antban commented Sep 7, 2016

Closes #56

```
# Linear timeout configuration
# initial timeout=300 seconds, after each failed start increase by 60 seconds (360, 420 and so on)
export STARTUP_TIMEOUT="type=linear:initial=300:step=60"
Copy link
Contributor

Choose a reason for hiding this comment

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

is that some standard format? For me it looks hacky

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, it's not standard, but I wanted to introduce a lot of different parameters...

Copy link
Contributor

Choose a reason for hiding this comment

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

STARTUP_TIMEOUT_TYPE
STARTUP_TIMEOUT_INITIAL
STARTUP_TIMEOUT_STEP

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@v-stepanov Well, my previous comment is really strange.
There are 2 types of startup timeout: linear and progressive. And they have different parameters - step and scale. I wanted to encapsulate this information to a single parameter, because STARTUP_TIMEOUT_STEP will not have any meaning for progressive timeout.

That means that STARTUP_TIMEOUT_STEP (SCALE) depends on STARTUP_TIMEOUT_TYPE. I wanted to have this information in one place, because having step and scale defined in the same time may lead to confusion, but having it defined in the same parameter won't have this problem, and it's simpler to understand what is the real configuration

Copy link
Contributor

@v-stepanov v-stepanov Sep 8, 2016

Choose a reason for hiding this comment

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

But you can just keep the name STARTUP_TIMEOUT_STEP for both modes.

I think having some custom complex structure for parameter value is very bad. I think the only thing that is acceptable is a list of items separate by ",".
And having parameters that depend on each other is fine for me, we can't really avoid that. At least they have the same prefix "STARTUP_TIMEOUT_"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@v-stepanov Ok. I agree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 194ae2e

@v-stepanov
Copy link
Contributor

Do you think it's already time to start writing integration tests? :)

@v-stepanov
Copy link
Contributor

In general :shipit:


class TestDataSizeStats(unittest.TestCase):
def test_linear_defaults(self):
o = StartupTimeout.build({'type': 'linear'})
Copy link
Contributor

Choose a reason for hiding this comment

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

is it an egg?

@antban antban merged commit dbf0297 into master Sep 9, 2016
@antban antban deleted the I56 branch September 9, 2016 11:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants