Updating waiters to use jmespath from io.burt #87
Conversation
| <groupId>software.amazon.awssdk</groupId> | ||
| <groupId>io.burt</groupId> | ||
| <artifactId>jmespath-jackson</artifactId> | ||
| <version>0.2.0</version> |
| @@ -81,51 +78,20 @@ | |||
| return javaWaiterModels; | |||
| } | |||
|
|
|||
| private JmesPathExpression getAstFromArgument(String argument, Map<String, JmesPathExpression> argumentToAstMap) | |||
| private Expression<JsonNode> getAstFromArgument(String argument, Map<String, Expression<JsonNode>> argumentToAstMap) | |||
There was a problem hiding this comment.
Do we even need this at all in the code gen anymore?
There was a problem hiding this comment.
Ah, actually we do not. Updated this first but then didn't end up needing it.
| return ast.accept(new JmesPathCodeGenVisitor(), null); | ||
| } | ||
| return null; | ||
| public Expression<JsonNode> getAst() { |
There was a problem hiding this comment.
I don't think this is used anymore.
| StringJoiner formatted = new StringJoiner("."); | ||
|
|
||
| for (String format : unformatted) { | ||
| formatted.add(StringUtils.uncapitalize(format)); |
There was a problem hiding this comment.
Does this handle the case we talked about where DBInstance should be capitalized to dbinstance?
There was a problem hiding this comment.
We fixed this in v2 with better and more consistent model naming. dbInstance in v2 vs dBInstance in v1
There was a problem hiding this comment.
Ah ok I was trying to wrap my head around how the heck this was even working then saw this, https://github.com/aws/aws-sdk-java-v2/blob/master/jmespath-java/src/main/java/software/amazon/awssdk/jmespath/ObjectMapperSingleton.java#L28
| @@ -83,9 +83,6 @@ public void setup() { | |||
| @After | |||
| public void tearDown() { | |||
| cf.deleteStack(DeleteStackRequest.builder().stackName(stackName).build()); | |||
| cf.waiters() | |||
There was a problem hiding this comment.
This is on the teardown method so don't think that we really need to wait for it to get deleted as long as the original request is received.
There was a problem hiding this comment.
Okay gotcha, I know for Dynamo there is a concurrent modification limit so it certain cases we do want to wait. But if the tests are consistently passing without this then I'd say leave if off.
spfink
force-pushed
the
finks/remove-waiters
branch
from
c8dace0
to
c8e7ab6
Compare
spfink
force-pushed
the
finks/remove-waiters
branch
from
c8e7ab6
to
bc0f105
Compare
While we still want to refactor waiters, this changes allows us to continue to use waiters in our tests and libraries while allowing us to no longer require python or the jmespath-java module that we maintained.