feat(server): make it possible to POST custom typings for testing during typing creation#1602
feat(server): make it possible to POST custom typings for testing during typing creation#1602
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
2835508 to
2f0d292
Compare
77c3c5f to
038cf30
Compare
2f0d292 to
015701a
Compare
038cf30 to
c318448
Compare
015701a to
d7c77ab
Compare
c318448 to
2d36b76
Compare
d7c77ab to
5e386d6
Compare
2d36b76 to
2b90ea5
Compare
5e386d6 to
aaf457e
Compare
2b90ea5 to
e0e929e
Compare
aaf457e to
65ffa66
Compare
e0e929e to
77a6d93
Compare
65ffa66 to
8c04f7d
Compare
79aa99e to
7fb6fca
Compare
6f967d4 to
eace48a
Compare
7fb6fca to
3fd2df2
Compare
eace48a to
d508bed
Compare
3fd2df2 to
bf416ed
Compare
d508bed to
86848dd
Compare
bf416ed to
5b9b0bd
Compare
86848dd to
665dcd3
Compare
5b9b0bd to
9df9c70
Compare
665dcd3 to
6fd7327
Compare
9df9c70 to
d4821a5
Compare
6fd7327 to
02098ce
Compare
d4821a5 to
453cc11
Compare
02098ce to
37375d9
Compare
453cc11 to
938690e
Compare
jit-binding-server/src/main/kotlin/io/github/typesafegithub/workflows/jitbindingserver/Main.kt
Outdated
Show resolved
Hide resolved
37375d9 to
c1dc24a
Compare
938690e to
6852b0d
Compare
c1dc24a to
6d56a0d
Compare
5340440 to
6d4a4ea
Compare
6d56a0d to
1dc7506
Compare
6d4a4ea to
7cb9a46
Compare
krzema12
left a comment
There was a problem hiding this comment.
Did the next pass, not fully reviewed. Moderately actionable on you.
| val version = call.parameters["version"]!! | ||
| val types = call.receiveText() | ||
| runCatching { | ||
| Load().loadOne(types) |
There was a problem hiding this comment.
How about making toBindingArtifacts communicate such case via its returned value? Then we'd avoid adding a direct dependency on snakeyam-engine-kmp, and generally having parsing these encapsulated in the function (not mentioning it's an abstraction leak because it's not realistic to have anything else than YAML here).
There was a problem hiding this comment.
Actually, I don't really agree.
The value is coming from the HTTP request, so is user input.
User input should always be considered to be not what you expect and be validated.
The body is an "argument" to the "API" and the "API" defines this has to be valid YAML.
So imho it is the right thing to explicitly validate the body to be valid YAML and also do that as soon as possible, not implicitly after several method calls two modules deep.
Also imho doing it that way would be the abstraction leak.
Because currently the place that parses the YAML is not aware that it is run behind an HTTP API.
Doing this change would need to make it aware that it is run behind an HTTP request, because it has to differentiate whether the types are coming as "API argument" so that it can result in "this is an unprocessable entity user error". If any of the other YAMLs has a parsing error, this is not a 4xx user error, as the user calling the HTTP API did not make an error, but it is a 5xx error as something on the server side is not working properly.
| fetchUri: (URI) -> String = ::fetchUri, | ||
| types: String? = null, |
There was a problem hiding this comment.
I think we can make it better - adding types looks like a carveout in this function, and implies some hacks across the project like adding typesUuid to ActionCoords. I'd expect that just parsing of the YAML is triggered if the types as string are provided. I'll think about how to structure it better.
There was a problem hiding this comment.
What do you mean by "carveout"?
It says "give me the types, here is the YAML to use, if I don't give one explicitly default to requesting them from the action or catalog". For me this is not a "carveout" unless I don't get what you mean.
I also do not really see what "hacks" you mean. The "typesUuid" for me is not a hack but a generated part of the coordinates. It also is part of the coordinates, it is just split once and then stored into a separate field.

With this you can for example do
curl --data-binary @action-types.yml https://bindings.krzeminski.it/pbrisbin/setup-tool-action/v2/setup-tool-action-v2.pomthen get as result coordinates containing a UUID in the group field which you can then use for an hour (or from the Maven Local cache thereafter) to test with
the supplied typing.