Conversation
If 3scale doesn't provide a request_id, or testing is done locally, we generate a request_id. We also only reply with the proper header containing that ID if it matches the leading zeroes format we have. The chances of uuid4 generating a uuid that matches this check is super low
payload_id key has been changed
| """ | ||
| mnm.uploads_total.inc() | ||
| self.request_id = self.request.headers.get('x-rh-insights-request-id', uuid.uuid4().hex) | ||
| self.request_id = self.request.headers.get('x-rh-insights-request-id', "00000" + uuid.uuid4().hex) |
There was a problem hiding this comment.
I don't see anything technically wrong with this. However, I do have a couple of suggestions:
-
You might consider making the prefix a "constant" ...something like "GENERATED_REQUEST_ID_PREFIX". You could then use this name in the code instead of the hardcoded string. I use constants/names because I have a tendency to "booger" up the strings by misspelling the words, missing a "0", etc.
-
You might consider moving the request_id generation into a method:
def generate_request_id():
return GENERATED_REQUEST_ID_PREFIX + uuid.uuid4().hex -
You might consider moving the retrieval of the x-rh-insights-request-id to a method:
def get_request_id(request):
return request.headers.get("x-rh-insights-request-id", generate_request_id())
I try to use method names (instead of comments) to try to end up with self-documenting code (that's the theory at least :) ). The other nice thing that comes out of splitting the logic into more (smaller) methods is containment and with containment...you end up with code that is easier to move around (when you figure out it doesn't belong where its currently located) and code that is easier to unit test.
|
|
||
| self.filedata = body | ||
|
|
||
| if self.request_id[:5] == "00000": |
There was a problem hiding this comment.
i don't see anything wrong with this code either.
The same suggestions apply. I would try to wrap the prefix comparison logic in a method. Basically I would try to make it more clear what the intentions of this code is.
2 participants
In the event that local testing is being done or 3scale doesn't assign an ID for some reason, we self-generate one. These are distinguished by having leading 0's to show that it's been self generated.
This stems from a conversation in https://projects.engineering.redhat.com/browse/RHCLOUD-516