You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains 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
The reason will be displayed to describe this comment to others. Learn more.
Hello! I was the author of #31, thanks for merging that. However, I think that this commit introduces some potential confusion. The idea of the CONF_API_KEY_HEADER_NAME is to provide the name of the header to look for to optionally override the value "Authorization" (some APIs require an alternative header name for the API key, e.g., WMATA wants "api_key" per #30). With this commit, it will now look for the header "Authorization" to contain that header name:
With the PR (before this commit), the relevant headers would be:
I think that using the same key "Authorization" to mean either the API key value or the custom header name could be confusing. If you understood all this and disagree, then fair enough, I won't argue -- it is your repository after all! Also, I will note that the PR was written a little while ago, so if I'm misremembering something and this change was needed then sorry! Either way, thanks for creating and maintaining this project.
The reason will be displayed to describe this comment to others. Learn more.
Yes, I already know that. I only did it a few minutes ago (at 5:00am). At least wait until I do a release for commenting.
From: elffaW ***@***.***>
Sent: Wednesday, May 8, 2024 5:29 AM
To: mark1foley/ha-gtfs-rt-v2 ***@***.***>
Cc: Subscribed ***@***.***>
Subject: Re: [mark1foley/ha-gtfs-rt-v2] Update sensor.py (2860792)
Hello! I was the author of #31<#31>, thanks for merging that. However, I think that this commit introduces some potential confusion. The idea of the CONF_API_KEY_HEADER_NAME is to provide the name of the header to look for to optionally override the value "Authorization" (some APIs require an alternative header name for the API key, e.g., WMATA wants "api_key" per #30<#30>). With this commit, it will now look for the header "Authorization" to contain that header name:
With the PR (before this commit), the relevant headers would be:
{
"api_key_header": "api_key",
"api_key": "abcd1234",
}
or with the default behavior:
{
"Authorization": "abcd1234",
}
After this commit, the relevant headers would be:
{
"Authorization": "api_key",
"api_key": "abcd1234",
}
(the default behavior is the same as above)
I think that using the same key "Authorization" to mean either the API key value or the custom header name could be confusing. If you understood all this and disagree, then fair enough, I won't argue -- it is your repository after all! Also, I will note that the PR was written a little while ago, so if I'm misremembering something and this change was needed then sorry! Either way, thanks for creating and maintaining this project.
—
Reply to this email directly, view it on GitHub<2860792#commitcomment-141778065>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AL6CAIGXRT35YZBKQ2BIQELZBETODAVCNFSM6AAAAABHLTLWQGVHI2DSMVQWIX3LMV43OQ3PNVWWS5CDN5WW2ZLOOQ5TCNBRG43TQMBWGU>.
You are receiving this because you are subscribed to this thread.Message ID: ***@***.******@***.***>>
2860792There 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.
Hello! I was the author of #31, thanks for merging that. However, I think that this commit introduces some potential confusion. The idea of the
CONF_API_KEY_HEADER_NAMEis to provide the name of the header to look for to optionally override the value"Authorization"(some APIs require an alternative header name for the API key, e.g., WMATA wants"api_key"per #30). With this commit, it will now look for the header "Authorization" to contain that header name:With the PR (before this commit), the relevant headers would be:
{ "api_key_header": "api_key", "api_key": "abcd1234", }or with the default behavior:
{ "Authorization": "abcd1234", }After this commit, the relevant headers would be:
{ "Authorization": "api_key", "api_key": "abcd1234", }(the default behavior is the same as above)
I think that using the same key
"Authorization"to mean either the API key value or the custom header name could be confusing. If you understood all this and disagree, then fair enough, I won't argue -- it is your repository after all! Also, I will note that the PR was written a little while ago, so if I'm misremembering something and this change was needed then sorry! Either way, thanks for creating and maintaining this project.2860792There 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.