dynamodb-toolbox
dynamodb-toolbox copied to clipboard
jeremydaly
Use incrementing ids for expression keys
Fixes https://github.com/jeremydaly/dynamodb-toolbox/issues/94
I've introduced a new ExpressionId class which will generate auto incrementing ids for expressions.
@jeremydaly if you're okay with this approach I can update the unit test assertions
@mitchheddles thanks for the PR, you're awesome!
I think that this approach is a bit problematic, debugging wise. I'll need to give this more thought.
Few months ago I started this PR, wdyt about that approach? if you like it, feel free to continue it, unfortunately I don't have enough time to finish it.
@mitchheddles thanks for the PR, you're awesome!
I think that this approach is a bit problematic, debugging wise. I'll need to give this more thought.
Are you referring to my first implementation with the find and replace, or the newest version?
Is your concern that attr0 is impossible to debug when an error is thrown? If so, we could do something similar to what you have but with an incrementing counter to ensue uniqueness.
@mitchheddles thanks for the PR, you're awesome!
I think that this approach is a bit problematic, debugging wise. I'll need to give this more thought.
Are you referring to my first implementation with the find and replace, or the newest version?
Is your concern that
attr0is impossible to debug when an error is thrown? If so, we could do something similar to what you have but with an incrementing counter to ensue uniqueness.
I'm referring to both. Regarding the counter, we need to keep in mind that the payload to DDB should be as minimal as possible so I'm not entirely sure we should do that. Let me know what you think
@naorpeled what if this was a configuration option that’s off by default? People can opt into it.
Alternatively, we might need custom error handling so we can show more detailed messaging.
@naorpeled what if this was a configuration option that’s off by default? People can opt into it.
Alternatively, we might need custom error handling so we can show more detailed messaging.
Hmm, would need to discuss this with @ThomasAribart and @jeremydaly.
@naorpeled please let me know what you decide, I’m keen to keep this one moving
@naorpeled please let me know what you decide, I’m keen to keep this one moving
Haven't received a reply yet, will keep you posted ❤️
Appreciate your energy, you rock!
@naorpeled please let me know what you decide, I’m keen to keep this one moving
Haven't received a reply yet, will keep you posted ❤️
Appreciate your energy, you rock!
Hey mate, has there been any news yet? We’re currently using a patched version of this library in production for a few weeks now and haven’t had any troubles. It would be great to get this released properly.