feat: add user-agent header middleware #87
Conversation
rcoh
left a comment
rcoh
left a comment
There was a problem hiding this comment.
if you haven't thought about it already, it's worth considering how you'll allow users (eg. HLLs) to customize your user agent
This is explicitly called out that we haven't done this but need to. The linked ticket is for that purpose. Good call out though. |
kggilmer
left a comment
kggilmer
left a comment
There was a problem hiding this comment.
A few questions but nothing found that should block pushing.
|
|
||
| override fun create(block: Config.() -> Unit): UserAgent { | ||
| val config = Config().apply(block) | ||
| val metadata = requireNotNull(config.metadata) { "metadata is required" } |
There was a problem hiding this comment.
question
I'm not sure where to look but in serde I know we require specific exception types. Is IllegalArgumentException what we want to throw in this case?
There was a problem hiding this comment.
so far I've been ok with throwing IllegalArgumentException from the standpoint that the only way this should really be happening is if we generated the code wrong. Whatever exception is thrown wouldn't matter because you can't do anything about it anyway, it would need fixed in codegen.
It's a good question though, I just don't have an answer that improves the situation in any measurable way
|
|
||
| internal actual fun platformLanguageMetadata(): LanguageMetadata { | ||
| val jvmMetadata = mutableMapOf<String, String>( | ||
| "javaVersion" to System.getProperty("java.version"), |
There was a problem hiding this comment.
In the case that some JVM doesn't return these values would be safer to guard the null and specify a default.
3 participants
Issue #, if available:
closes #85
Description of changes:
X-Amz-User-AgentandUser-Agentheaders (the former is the new SEP)smithy-build.jsonThis PR implements most but not all of the SEP. There are a few optional metadata types left and we will need to figure out how to allow callers to add to/extend this metadata (e.g. amplify would be a good candidate for setting
lib/*type metadata). See #86By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.