Skip to content

Conversation

@aranaea
Copy link
Contributor

@aranaea aranaea commented Nov 1, 2017

I was trying to test out the schema-registry and confluent python libraries and ran into a couple of issues with the README. Just thought I would send them your way.

The main issue was that I received a 409 when trying to run the producer.

confluent_kafka.avro.error.ClientError: Incompatible Avro schema:409

I was able to post the same schema to the registry directly, so I knew the schema was actually valid but noticed that in my manual post I had used a different topic name with no '' characters. When I changed this is the sample code everything worked fine. My assumption is that '' is not a valid character as a subject in the schema registry.

I've also included 2 other small changes to the example code. First, I included the schemas as strings in the producer example so that people could just cut and paste without creating the schema separately.

Second, I moved the schema's to be passed in to the call to produce instead of as default schemas. This was because a colleague immediately asked about the ability to send multiple message types with associated schemas when I showed her the example code. I just thought this made that capability more clear.

@ghost
Copy link

ghost commented Nov 1, 2017

It looks like @aranaea hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement here.

Once you've signed reply with [clabot:check] to prove it.

Appreciation of efforts,

clabot

@aranaea
Copy link
Contributor Author

aranaea commented Nov 1, 2017

[clabot:check]

@ghost
Copy link

ghost commented Nov 1, 2017

@confluentinc It looks like @aranaea just signed our Contributor License Agreement. 👍

Always at your service,

clabot

@aranaea
Copy link
Contributor Author

aranaea commented Nov 1, 2017

Tests seem to be failing on something unrelated

1.46s$ if [[ -z $CIBW_BEFORE_BUILD ]]; then flake8 ; fi
./confluent_kafka/avro/cached_schema_registry_client.py:171:13: E722 do not use bare except'
./confluent_kafka/avro/cached_schema_registry_client.py:207:13: E722 do not use bare except'
./confluent_kafka/avro/cached_schema_registry_client.py:272:9: E722 do not use bare except'
./confluent_kafka/avro/serializer/message_serializer.py:45:1: E722 do not use bare except'
./confluent_kafka/avro/serializer/message_serializer.py:161:9: E722 do not use bare except'
./confluent_kafka/avro/serializer/message_serializer.py:183:13: E722 do not use bare except'
./examples/consumer.py:62:9: E722 do not use bare except'
./tests/test_threads.py:8:1: E722 do not use bare except'
./tests/avro/mock_registry.py:123:9: E722 do not use bare except'
The command "if [[ -z $CIBW_BEFORE_BUILD ]]; then flake8 ; fi" exited with 1.

@edenhill
Copy link
Contributor

edenhill commented Nov 1, 2017

I think a newer version of flake8 added more checks, you can ignore it (or file a PR ;) )

README.md Outdated

avroProducer = AvroProducer({'bootstrap.servers': 'mybroker,mybroker2', 'schema.registry.url': 'http://schem_registry_host:port'}, default_key_schema=key_schema, default_value_schema=value_schema)
avroProducer.produce(topic='my_topic', value=value, key=key)
avroProducer = AvroProducer({'bootstrap.servers': 'mybroker,mybroker2', 'schema.registry.url': 'http://schem_registry_host:port'})
Copy link
Contributor

Choose a reason for hiding this comment

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

typo (not yours) : schem_registry_host (missing an a)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll update it 👍

key = {"name": "Key"}

avroProducer = AvroProducer({'bootstrap.servers': 'mybroker,mybroker2', 'schema.registry.url': 'http://schem_registry_host:port'}, default_key_schema=key_schema, default_value_schema=value_schema)
avroProducer.produce(topic='my_topic', value=value, key=key)
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove default_*_schema args in favour of per-produce-call schema args?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first thing I was looking to do was send a few messages on a topic. But my topics have multiple messages per topic, so the default_*_schema didn't work for me. I poked around a bit before finding the parameters to the produce call so wanted to capture it in the doc. I'm not sure what the most common use case is but I thought passing the schemas on the produce call was the most flexible.

I'm happy to change this back if you think it makes more sense to use the default schemas.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the delay, I think you should change it back to using default schema, since that is what most people will likely be using.

value_schema = avro.load('ValueSchema.avsc')
key_schema = avro.load('KeySchema.avsc')

value_schema_str = """
Copy link
Contributor

Choose a reason for hiding this comment

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

While this is somewhat verbose it is indeed nice with a self-contained examply 👍

Copy link
Contributor

@edenhill edenhill left a comment

Choose a reason for hiding this comment

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

LGTM, just need to change back to default schemas.

key = {"name": "Key"}

avroProducer = AvroProducer({'bootstrap.servers': 'mybroker,mybroker2', 'schema.registry.url': 'http://schem_registry_host:port'}, default_key_schema=key_schema, default_value_schema=value_schema)
avroProducer.produce(topic='my_topic', value=value, key=key)
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the delay, I think you should change it back to using default schema, since that is what most people will likely be using.

@aranaea
Copy link
Contributor Author

aranaea commented Mar 27, 2018

Sorry for the long delay. I've updated the README to include the default schemas. I'm back around se let me know if you want anything else updated.

@edenhill edenhill merged commit 98af46d into confluentinc:master Mar 29, 2018
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.

2 participants