Add git support for compatibility checker #17684
Conversation
github-actions
bot
added
tools
build
generator/src/main/java/org/apache/kafka/message/checker/CheckerUtils.java
Show resolved
Hide resolved
generator/src/main/java/org/apache/kafka/message/checker/CheckerUtils.java
Outdated
Show resolved
Hide resolved
generator/src/main/java/org/apache/kafka/message/checker/CheckerUtils.java
Outdated
Show resolved
Hide resolved
generator/src/main/java/org/apache/kafka/message/checker/MetadataSchemaCheckerTool.java
Outdated
Show resolved
Hide resolved
| String fileCheckMetadata = namespace.getString("file"); | ||
| String gitContent = GetDataFromGit(fileCheckMetadata); | ||
| EvolutionVerifier verifier = new EvolutionVerifier( | ||
| CheckerUtils.readMessageSpecFromFile(Paths.get("").toAbsolutePath().getParent() + "/metadata/src/main/resources/common/metadata/" + fileCheckMetadata), |
There was a problem hiding this comment.
in general I don't understand what the strategy is here for paths. if you are using a relative path, you need to document what directory we need to be in. how are we locating the git repository root?
There was a problem hiding this comment.
Ok yeah I added documentation in the help output stating that this tool is expected to be run from the generator directory. That was the assumption I was working under. Getting the git root repo is done in the GetDataFromGit function in CheckerUtil.java
There was a problem hiding this comment.
If we know where the git repo root is, why can't we just change to that directory rather than requiring a specific current working directory?
There was a problem hiding this comment.
I changed to code so it works anywhere within the kafka directory. Im trying to get the absolute path to the kafka directory so that I can use that to git repo through jgit
checkstyle/suppressions.xml
Outdated
| @@ -32,6 +32,8 @@ | |||
| files="(ApiMessageType|FieldSpec|MessageDataGenerator|KafkaConsumerTest).java"/> | |||
| <suppress checks="MethodLength" | |||
| files="(FieldSpec|MessageDataGenerator).java"/> | |||
| <suppress checks="ImportControl" | |||
There was a problem hiding this comment.
why can we not just list org.eclipse.jgit as allowed?
There was a problem hiding this comment.
Was having trouble testing to so added it in the suppressions. Figured out I was having a local issue so this is just added to the allowed import controls now.
|
@mannoopj : there are checkstyle issues, can you fix? |
Add git support for schema compatibility checker. Pulls in valid schema from remote git trunk branch to check with edited schema in local branch. Adds new option for command line
verify-evolution-gitwhich takes in a required file name