Skip to content

Conversation

jasonzio
Copy link

If the django-guid package is installed and django_guid.set_guid() has been called, fetch that guid and apply it as the correlationId for all Azure storage API calls. If the module isn't importable or no guid has been set, don't supply a correlationId.

While the test environment requires the django-guid package, the runtime environment need not have that package installed.


def _make_headers():
headers = {}
correlation_id = get_guid()

Choose a reason for hiding this comment

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

Is committing to django-guid for this something we want? What if people have other means of generating a Correlation ID (django-log-request-id or OpenTelemetry).

Maybe simple adding a make_headers method to the Azure Storage, and allowing users to override it using any solution they want, would be a simpler solution, without committing to any individual package.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's PRs open that add a request_options method to do this.

Copy link

Choose a reason for hiding this comment

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

@hartungstenio We are still very interested in getting one of the PRs that resolves this issue merged, please let me know what changes you require. Currently there is no way to pass through the per-operation keyword args to the Azure Storage client, except for timeout which is already explicitly added on each request. Merging one of the PRs that create the AZURE_REQUEST_OPTIONS setting gives users the ability to set these per-request options, if they care about them.

It is my personal opinion that using django-guid if it is installed is the most-correct thing to do, and so #1468 is the best candidate. However if you don't want to be tied to django-guid at all then #1467 is also acceptable.

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.

4 participants