docs: Add docs for all Django settings #126
Conversation
timmc-edx
force-pushed
the
timmc/doc-settings
branch
2 times, most recently
from
de0abab to
f026f04
Compare
timmc-edx
force-pushed
the
timmc/doc-settings
branch
from
f026f04 to
309e1d4
Compare
| url = getattr(settings, 'EVENT_BUS_KAFKA_SCHEMA_REGISTRY_URL', None) | ||
| if url is None: | ||
| warnings.warn("Cannot configure event-bus-kafka: Missing setting EVENT_BUS_KAFKA_SCHEMA_REGISTRY_URL") | ||
| return None | ||
|
|
||
| # .. setting_name: EVENT_BUS_KAFKA_SCHEMA_REGISTRY_API_KEY | ||
| # .. setting_default: None | ||
| # .. setting_description: API key for talking to the Avro schema registry specified in |
There was a problem hiding this comment.
This key and secret are optional as well (figure we should probably be consistent with how we describe the Kafka API key)
There was a problem hiding this comment.
Oh! Yes, I was wondering about this and forgot to leave myself a note. The Kafka API key/secret are definitely optional because if either one is omitted, we skip adding both of them to the config. But with these schema registry values, we always use them, even if they're None or empty. So for instance we could end up with a basic.auth.user.info of :, user:, :pass, or user:pass. Not all of those look particularly sensible. I'll go check the schema registry docs again...
(Also, I see that I've documented them as defaulting to None but they're actually ''. Oops.)
There was a problem hiding this comment.
Fixed the docs of the defaults. OK, here's what I've got so far:
- https://docs.confluent.io/platform/current/clients/confluent-kafka-python/html/index.html#schemaregistryclient describes
basic.auth.user.infoas an optional field (it does not have the*that marks required fields) and says that it must be inuser:passformat - https://github.com/confluentinc/confluent-kafka-python/blob/v2.0.2/src/confluent_kafka/schema_registry/schema_registry_client.py#L110 is the implementation we use (there's also a cached client I haven't looked into much) and the code corroborates that description
I guess what I'm unsure of is this: What happens if the schema-registry server is configured not to require auth, and the client uses the defaults, i.e. ':'? Is that going to cause an issue?
There was a problem hiding this comment.
It looks like it will look to the url first for auth information and then error if there is also basic.auth.user.info. I vote we punt on what happens if the schema registry server doesn't expect any auth and just put in docs somewhere that user should use the settings provided instead of putting the user and password in the URL
There was a problem hiding this comment.
Yeah, we should probably document that auth goes in the auth settings and not in the URL. But what to do about required/optional? I don't want to make claims in the docstrings that we can't back up, and so I'm tempted to just... not mention required/optional either way, for those two.
There was a problem hiding this comment.
I think "Optional" still makes sense because we do know for sure that you don't have to always have to set them.
There was a problem hiding this comment.
Hmm. I still don't know what happens if the schema registry isn't configured for auth and we pass an auth header containing ':' anyway. Maybe nothing? I guess if it causes any issues then we can put in a bugfix when we find out.
Addresses #124
Merge checklist:
Check off if complete or not applicable: