Skip to content

Handle proprietary headers in spec summary. - #57

Draft
mgritter wants to merge 1 commit into
mainfrom
mgritter/handle_variable_auth_types
Draft

Handle proprietary headers in spec summary.#57
mgritter wants to merge 1 commit into
mainfrom
mgritter/handle_variable_auth_types

Conversation

@mgritter

@mgritter mgritter commented Jul 9, 2021

Copy link
Copy Markdown
Contributor

First of the downstream changes handling the IR change in postmanlabs/observability-ir#2.

Probably they need to go in together; I'll do the CLI changes next.

@mgritter
mgritter requested review from liujed and thatplguy July 9, 2021 23:31

@liujed liujed left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here's another place where authorization types are read. I think this might also need to be adjusted. https://github.com/akitasoftware/akita-libs/blob/8388976a7962059b6e7c2f1fe60d0700f62d4e4a/visitors/http_rest/spec_visitor.go#L488-L493

@mgritter

Copy link
Copy Markdown
Contributor Author

Here's another place where authorization types are read. I think this might also need to be adjusted.

https://github.com/akitasoftware/akita-libs/blob/8388976a7962059b6e7c2f1fe60d0700f62d4e4a/visitors/http_rest/spec_visitor.go#L488-L493

Do you think the context needs to be extended with the header name? Because as things stand, there's nothing more I can do in that case.

@liujed

liujed commented Jul 12, 2021

Copy link
Copy Markdown
Contributor

I'm not sure what we should be doing. Maybe have named be true and set name to the name of the underlying header, now that it's not just "Authorization". We'd have to check all the places where this is used to make sure the name is no longer assumed to be "Authorization".

@mgritter
mgritter marked this pull request as draft March 9, 2023 17:19
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