Skip to content
This repository has been archived by the owner on Aug 23, 2023. It is now read-only.

Rm old mt index tools and mt-replicator #783

Merged
merged 11 commits into from
Dec 14, 2017
Merged

Conversation

Dieterbe
Copy link
Contributor

@Dieterbe Dieterbe commented Dec 13, 2017

I expect the new scripts/check_docs_uptodate.sh will break the build, because we have a lot of accumulated out-of-date-ness (unrelated to this PR) that we have to address

mt-replicator uses the sarama-cluster for distributing partitions
between instances and tracking offsets but it does not work reliably.
We have tried to use mt-replicator before and it regularly just stops
consuming from some partitions.
@Dieterbe Dieterbe changed the title Rm old mt index tools Rm old mt index tools and mt-replicator Dec 14, 2017
@Dieterbe Dieterbe requested a review from replay December 14, 2017 13:04
./config-to-doc.sh > $tmp
diff ../docs/config.md $tmp

# metrics2docs .> docs/metrics.md this doesn't work very well yet
Copy link
Contributor

Choose a reason for hiding this comment

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

you might want to rm $tmp

exit 2
fi
./config-to-doc.sh > $tmp
diff ../docs/config.md $tmp
Copy link
Contributor

Choose a reason for hiding this comment

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

looks unfinished

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what do you mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

did you mean $tmp removal also here?

Copy link
Contributor

@replay replay Dec 14, 2017

Choose a reason for hiding this comment

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

it doesn't do anything with the returned value. or is it supposed to just output the diff and nothing else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it outputs the diff, if any, and the script exits with the exit code of the diff command, so that we can verify it. but to your point, i will add the rm $tmp which requires adding the explicit exiting with diff's exit code (which currently is not obvious to the reader)

@Dieterbe
Copy link
Contributor Author

@replay fixed

@woodsaj
Copy link
Member

woodsaj commented Dec 14, 2017

LGTM

@Dieterbe Dieterbe merged commit e833430 into master Dec 14, 2017
@Dieterbe Dieterbe deleted the rm-old-mt-index-tools branch December 15, 2017 19:48
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