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

refactor: improve kafka-check-schemas commit hook #34

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

yannrouillard
Copy link
Contributor

Creating this PR to open the discussion about how to improve the pre-commit hook for checking kafka schemas.

The current hooks works by calling a specific target in scala projects, generateKafkaSchemas, and checking if this leads to a schemas file different or not (which would mean the schemas is not properly updated)

This works but has some disadvantages:

  • the pre-commit run sbt which is slow to start so this slows down the pre-commit execution. This is mitigated by the fact that it only runs when the src/../models folder is updated but this is not entirely satisfying
  • when a schema is not up to date, it must even be run twice: one when pre-commit discovers that the schemas are not up to date and one when we have commited the updated files as the pre-commit will check again.

Basically, we want the schemas generation to be fast and not get in the way of the developers, but we also would like to detect and warn the devs if for some reason these schemas are not up to date anymore when they commit (either they forgot to commit the new schemas files or the schemas update mechanism doesn't work anymore).

One option that has been mentioned is to include that step in the compile stage so that no time in lost in multiple sbt executions. This means the schemas files would be refreshed at each compilation and we would need to find a way to check if the committed ones have been refreshed.

@yannrouillard
Copy link
Contributor Author

@Preetwinder @leo-dur @sebguillaume-kpler @ALB5 inviting you on this PR so we can discuss better options

@yannrouillard
Copy link
Contributor Author

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.

1 participant