Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

amazonka-dynamodb: TransactWriteItem should be a sum type #1006

Open
endgame opened this issue Sep 18, 2024 · 2 comments
Open

amazonka-dynamodb: TransactWriteItem should be a sum type #1006

endgame opened this issue Sep 18, 2024 · 2 comments
Milestone

Comments

@endgame
Copy link
Collaborator

endgame commented Sep 18, 2024

TransactWriteItem is a product of four Maybes. It should be a sum type like AttributeValue, that would be much easier to work with.

I think it should be relatively simple and just need a PR similar to #799 or #724 .

Running scripts/generate --commit dynamodb will regenerate the service binding.

@endgame endgame added this to the 2.1 milestone Sep 18, 2024
@ulidtko
Copy link
Contributor

ulidtko commented Oct 2, 2024

Hi @endgame , I took a stab at this. Didn't commit yet, got this far:

Building library for amazonka-dynamodb-2.0..
Module graph contains a cycle:
        module ‘Amazonka.DynamoDB.Types.ReturnValuesOnConditionCheckFailure’ (gen/Amazonka/DynamoDB/Types/ReturnValuesOnConditionCheckFailure.hs)
        imports module ‘Amazonka.DynamoDB.Types.TransactWriteItem’ (src/Amazonka/DynamoDB/Types/TransactWriteItem.hs)
  which imports module ‘Amazonka.DynamoDB.Types.ConditionCheck’ (gen/Amazonka/DynamoDB/Types/ConditionCheck.hs)
  which imports module ‘Amazonka.DynamoDB.Types.ReturnValuesOnConditionCheckFailure’ (gen/Amazonka/DynamoDB/Types/ReturnValuesOnConditionCheckFailure.hs)

The imports in generated modules are completely unnecessary, and removing them by hand gives me a successful cabal build amazonka-dynamodb. I guess I must've botched the generator config somehow:

--- i/configs/services/dynamodb.json
+++ w/configs/services/dynamodb.json
@@ -10,6 +10,7 @@
   ],
   "typeModules": [
     "Amazonka.DynamoDB.Types.AttributeValue",
+    "Amazonka.DynamoDB.Types.TransactWriteItem",
     "Amazonka.DynamoDB.Types.WriteRequest"
   ],
   "typeOverrides": {
@@ -40,6 +41,12 @@
     "QueryOutput": {
       "requiredFields": ["Items"]
     },
+    "TransactWriteItem": {
+      "replacedBy": {
+        "name": "TransactWriteItem",
+        "underive": []
+      }
+    },
     "WriteRequest": {
       "replacedBy": {
         "name": "WriteRequest",

Any advise?.. The new hand-written module must import Amazonka.DynamoDB.Types.{ConditionCheck,Delete,Put,Update} which are generated, but with unnecessary reverse-direction import Amazonka.DynamoDB.Types.TransactWriteItem, for some reason. 🤔

Otherwise, I think I'll have enough time to open ~soon a mergeable PR that implements this. The core idea is there:

data TransactWriteItem
  = ConditionCheck ConditionCheck
  | Delete Delete
  | Put Put
  | Update Update

TL;DR: the issue is clear to me; tomorrow I'll open a draft PR that almost-implements it, modulo an other issue with extra imports getting generated that create import cycles.

@endgame
Copy link
Collaborator Author

endgame commented Oct 2, 2024

It will require changes to the generator, which currently imports all typeModules in every file. I see a couple of options:

  1. Add a generator option to generate .hs-boot files for selected types, and then import {-# SOURCE #-} them from your hand-written module.
  2. Add a generator option to exclude certain imports from specific generated modules.
  3. Disable generation of ConditionCheck/Delete/Put/Update and inline them into TransactWriteItem.hs.

№1 feels more flexible in general, though for it to work in all cases it would need to know to {-# SOURCE #-}-import any modules that also have a .hs-boot version. №2 seems a bit brittle. №3 I really don't like because we won't pick up new features if AWS extends the operations again, but I wanted to list it for completeness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants