gharris1727 commented on PR #14064: URL: https://github.com/apache/kafka/pull/14064#issuecomment-1668640505
> I think our workaround with a special path of classpath to denote classpath plugins in PluginUtils::classpathPluginSource should be improved. I built the basic auth extension, created a directory named classpath, moved the JAR file for the extension into that directory, and then ran bin/connect-plugin-path.sh list --plugin-location classpath. The end result was fairly confusing: Thanks for finding that, I've changed it so that PluginSource uses `null` as a sentinel value for the classpath, and changed all of the places it's printed to re-inject the `"classpath"` string after all the control flow has used `null`. I've replaced the makeUnique hack with Lists and counting instead of trying to make the ManifestEntry objects completely unique. > I've also finally gotten around to looking at the tests. It seems like we have a great foundation of scenarios to build on, but the assertions we're making are fairly minor. Can we add some coverage to verify, e.g., how many plugin rows are listed, the (partial or complete) contents of the post-table summary, and which plugins are described as being migrated vs. non-migrated? Yes the assertions are pretty sparse. I've been avoiding parsing the output while it was still in flux, but I think the format is starting to settle down now. I'll go through and see what I can add. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
