-
-
Notifications
You must be signed in to change notification settings - Fork 886
[Azure] Supply guid from django_guid as the correlationId on storage calls #1178
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
Open
jasonzio
wants to merge
8
commits into
jschneier:master
Choose a base branch
from
jasonzio:correlationid
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
ce1c5f0
Use django_guid to fetch and apply an Azure correlationId
jasonzio a811924
Add tests for django_guid passthru to Azure correlationId
jasonzio 97cd122
Mock correct target; fix typo'd keyword param; add mock param
jasonzio 102eb53
Revert tox.ini commands
jasonzio 79f9179
Fix flake8 issue (one too many blank lines)
jasonzio e2581af
Apply isort
jasonzio ce550ca
Merge upstream changes for use of custom URL for storage
jasonzio c4f556a
Merge branch 'master' into correlationid
sdherr File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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_headersmethod 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.There was a problem hiding this comment.
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_optionsmethod to do this.There was a problem hiding this comment.
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
timeoutwhich 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.