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

Updated classpath in schema-registry-run-class to reflect changes in … #257

Merged
merged 1 commit into from Nov 30, 2015

Conversation

granders
Copy link
Contributor

@miguno Mind taking a quick look? I noticed that the schema registry start script was failing due to SchemaRegistryMain no longer being in the classpath.

My primary question here is whether this is enough, or should we add other modules?
E.g. in pom.xml I now see

        <module>core</module>
        <module>client</module>
        <module>avro-serializer</module>
        <module>json-serializer</module>
        <module>avro-converter</module>
        <module>package-schema-registry</module>
        <module>package-kafka-serde-tools</module>

@ConfluentJenkins
Copy link
Collaborator

Refer to this link for build results (access rights to CI server needed):
http://jenkins.confluent.io/job/schema-registry-pr/69/
Test PASSed.

@miguno
Copy link
Contributor

miguno commented Nov 30, 2015

LGTM.

I assume you sent this PR because schema registry caused issues for muckrake? The packaged (e.g. rpm) version of schema registry works fine without this PR because it pulls in jars via a different for-loop in schema-registry-run-class.

FYI: I'll merge your PR but will keep this thread open for the follow-up discussion.

My primary question here is whether this is enough, or should we add other modules?

The jar directory you updated in your PR includes the code of the following modules (I skip the package-* modules as these don't contain code themselves):

        <module>core</module>
        <module>client</module>

What is not included is:

        <module>avro-serializer</module>
        <module>json-serializer</module>
        <module>avro-converter</module>

My question is now quite similar to yours: Does schema-registry at run-time (think: production) need any of the Avro/JSON serdes depending on the user's setup? Neither core/pom.xml nor client/pom.xml do require the serdes at compile time. /cc @ewencp .

miguno added a commit that referenced this pull request Nov 30, 2015
Updated classpath in schema-registry-run-class to reflect changes in …
@miguno miguno merged commit a2a7e08 into master Nov 30, 2015
@miguno miguno added the bug label Nov 30, 2015
@miguno miguno added this to the Second Release milestone Nov 30, 2015
@miguno miguno self-assigned this Nov 30, 2015
@miguno
Copy link
Contributor

miguno commented Nov 30, 2015

It looks like we don't need any further changes, @granders .

@ewencp ewencp deleted the fix-run-class branch May 20, 2016 00:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants