WIP Tracefile creation #204
Conversation
et22
force-pushed
the
tracefile_naming
branch
3 times, most recently
from
939599c to
a719663
Compare
...pescript-codegen/src/main/java/software/amazon/smithy/typescript/codegen/CodegenVisitor.java
Show resolved
Hide resolved
| @@ -372,6 +422,10 @@ private void generateClient(ServiceShape shape) { | |||
| boolean hasPaginatedOperation = false; | |||
|
|
|||
| for (OperationShape operation : containedOperations) { | |||
| OperationShape finalOperation = operation; | |||
There was a problem hiding this comment.
This new block seems like it might not be right.
There was a problem hiding this comment.
I removed the redundant variable and the block.
| @@ -18,6 +18,7 @@ extra["displayName"] = "Smithy :: Typescript :: Codegen" | |||
| extra["moduleName"] = "software.amazon.smithy.typescript.codegen" | |||
|
|
|||
| dependencies { | |||
| api("software.amazon.smithy:smithy-aws-traits:[1.7.2, 2.0[") | |||
There was a problem hiding this comment.
I think we're looking to avoid taking this dependency in this package. It'll require changing the resolution of the serviceId for creating a TraceMetadata in the CodegenVisitor, but seems like the right call.
There was a problem hiding this comment.
Yes, that makes sense. Updated to remove the dependency and this will now just use the service name.
| return Symbol.builder().putProperty("shape", shape).name(typeName); | ||
| return Symbol.builder() | ||
| .putProperty("shape", shape) | ||
| .putProperty("traceFileNamespace", moduleNameDelegator.formatModuleName(shape, null)) |
There was a problem hiding this comment.
These calls to the moduleNameDelegator are causing issues for the model chunking. This needs address before the PR can be merged. Testing with regenerating an existing client should be performed to assure that the model chunking isn't changed unnecessarily for an existing client.
| } | ||
|
|
||
| private Symbol.Builder createSymbolBuilder(Shape shape, String typeName, String namespace) { | ||
| return Symbol.builder() | ||
| .putProperty("shape", shape) | ||
| .name(typeName) | ||
| .namespace(namespace, "/"); | ||
| .namespace(namespace, "/") | ||
| .putProperty("traceFileNamespace", moduleNameDelegator.formatModuleName(shape, null)) |
There was a problem hiding this comment.
See comment above. Another call to moduleNameDelegator that should be addressed.
|
Can you run codegeneration? It's hard to see what changes this results in |
| @@ -212,6 +249,13 @@ void execute() { | |||
| settings, model, protocol, symbolProvider, writers, protocolGenerator).run(); | |||
| } | |||
|
|
|||
| // Write the TraceFile. | |||
| TracingSymbolProvider traceProvider = (TracingSymbolProvider) symbolProvider; | |||
| String traceName = symbolProvider.toSymbol(service).getName().replace("Client", "") | |||
There was a problem hiding this comment.
Notable that the replace here won't do anything for ssdks. That's probably fine though because we don't want them to share a filename, even if they can't be generated in the same plugin.
| Symbol targetSymbol = toSymbol(targetShape) | ||
| .toBuilder() | ||
| .putProperty("model", model) | ||
| .putProperty("SymbolProvider", this) |
There was a problem hiding this comment.
Is this safe? Any plugin that wraps the symbol provider could trip this up. Ideally they wouldn't, since they shouldn't touch members or unions, but it could become an issue.
| .addTag(TypeScriptShapeLinkProvider.REQUEST_TAG, "AWS SDK request type") | ||
| .addTag(TypeScriptShapeLinkProvider.RESPONSE_TAG, "AWS SDK response type") |
There was a problem hiding this comment.
| .addTag(TypeScriptShapeLinkProvider.REQUEST_TAG, "AWS SDK request type") | |
| .addTag(TypeScriptShapeLinkProvider.RESPONSE_TAG, "AWS SDK response type") | |
| .addTag(TypeScriptShapeLinkProvider.REQUEST_TAG, "Smithy client request type") | |
| .addTag(TypeScriptShapeLinkProvider.RESPONSE_TAG, "Smithy client response type") |
Issue #, if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.