question: Any chance for transaction support?

[stupoid]

I'm wondering if there are any plans to support TransactWriteItems or just Transactions in general.

I can probably work on this if/when I have more time 🤔

[ojii]

No plans to add support right now, but would be nice to have of course.

sadly, transactions seem to be unsupported by dynalite (or alternative dynamodb emulators) which will make testing this difficult (basically requiring "real" dynamodb).

We should probably discuss API design before you dive too deep. At a first glance, I think TransactItems should probably built similar to UpdateExpression, using & to combine different operations (put, update,...) into a single TransactItems object.

Maybe the signature should be transact_write_items(self, table: TableName, items: TransactItems, *, request_token: Optional[str] = None) -> None:, what do you think?

[dimaqq]

@stupoid do you use either transaction mechanism already?
if not, please read up on the limitations, they are hilarious :)

[dimaqq]

Calling on users who want transactions... please let yourselves known, e.g. by 👍🏿 on the OP ⬆️

[nicolaszein]

Hi @ojii, how are you?

I'm actually also interested in transaction support.

If you are still interested, I have the time to collaborate with this. 😄
About the dynalite not supporting transactions I have cloned the repo and ran the test suit against a localstack dynamodb and all the tests succeed. So I think it is an option what do you think?

And about the API design, I liked the idea of building similar to UpdateExpression. But I have some doubts about how to implement it. UpdateExpression is a collection of F expressions right? Do you think that TransactItems should be a collection of TransactItem?

I just want to hear your thoughts about and if possible collaborate with this. 😃

[ojii]

About the dynalite not supporting transactions I have cloned the repo and ran the test suit against a localstack dynamodb and all the tests succeed. So I think it is an option what do you think?

looks like localstack is a wrapper around dynamodb-local fixing some of its shortcomings. This seems like a reasonable thing to test against. So a good first step to adding transaction support would be to update our CI setup to also test against localstack. We can then either use feature detection or flag in the tests to only run transaction tests against implementations that support it.

Once we have something in place that we can test against, we can think about a good API for this. TransactGetItems should be fairly simple, TranactWriteItems is a bit trickier, but as you pointed out we should be able to re-use the expression building features in aiodynamo for things like ConditionCheck and Update.

[nicolaszein]

@ojii Nice! I think we can start with the CI.

How can we do that? Should I open a pull request adding a new step on the pipeline to run tests with localstack? 🤔

[ojii]

How can we do that? Should I open a pull request adding a new step on the pipeline to run tests with localstack? 🤔

yes please

[ojii]

And about the API design, I liked the idea of building similar to UpdateExpression. But I have some doubts about how to implement it. UpdateExpression is a collection of F expressions right?

not quite. UpdateExpression are built by calling methods on F objects and then optionally combining them together with &. This should be usable for the "update" part of TWI.

Conditions are also built using (different) methods on F objects (and combining them with & and/or |). This can be used for ConditionCheck both at the top level and in Put, Update and Delete. An interesting thing there is that for TWI I'm pretty sure we have to separate the expression names/values for each item so we have to be careful there.

Do you think that TransactItems should be a collection of TransactItem?

TransactItem should probably be a Union[ConditionCheck, Put, Update, Delete], but I'd have to actually see how this all looks in code to make an informed decision about the design here.

[nicolaszein]

Hi @ojii, thanks for your reply.

I opened the #131.
I also realize that there is a step with amazon/dynamodb-local and I think transactions work with that. Do you think that is also necessary localstack? 🤔

[ojii]

I also realize that there is a step with amazon/dynamodb-local and I think transactions work with that. Do you think that is also necessary localstack? 🤔

well, since you already did the PR I think it is reasonable to add localstack to the test matrix.

[nicolaszein]

Hi @ojii, thanks for merging the PR.

So, following your ideas I started to sketch a solution here and before I open another pull request I want to share with you.
It would be basically something like this:

@dataclass(frozen=True)
class Put:
    item: Item
    condition: Optional[Condition] = None

    def to_request_payload(self, table: TableName) -> Dict[str, Any]:
        dynamo_item = py2dy(self.item)
        if not dynamo_item:
            raise EmptyItem()

        payload: Dict[str, Any] = {
            "TableName": table,
            "Item": dynamo_item,
        }
        if self.condition:
            params = Parameters()
            payload["ConditionExpression"] = self.condition.encode(params)
            payload.update(params.to_request_payload())

        return payload


@dataclass(frozen=True)
class Update:
    key: Item
    expression: Expression
    condition: Optional[Condition] = None

    def to_request_payload(self, table: TableName) -> Dict[str, Any]:
        params = Parameters()
        expression = self.expression.encode(params)
        if not expression:
            raise EmptyItem()

        payload: Dict[str, Any] = {
            "TableName": table,
            "UpdateExpression": expression,
            "Key": py2dy(self.key),
        }

        if self.condition:
            payload["ConditionExpression"] = self.condition.encode(params)

        payload.update(params.to_request_payload())

        return payload


@dataclass(frozen=True)
class Delete:
    key: Item
    condition: Optional[Condition] = None

    def to_request_payload(self, table: TableName) -> Dict[str, Any]:
        dynamo_item = py2dy(self.key)
        if not dynamo_item:
            raise EmptyItem()

        payload: Dict[str, Any] = {
            "TableName": table,
            "Key": dynamo_item,
        }
        if self.condition:
            params = Parameters()
            payload["ConditionExpression"] = self.condition.encode(params)
            payload.update(params.to_request_payload())

        return payload


@dataclass(frozen=True)
class TransactWriteItems:
    items_to_put: Optional[List[Put]] = None
    items_to_update: Optional[List[Update]] = None
    items_to_delete: Optional[List[Update]] = None

    def to_request_payload(self, table: TableName) -> List[Dict[str, Any]]:
        payload: List[Dict[str, Any]] = []
        if self.items_to_put:
            payload.extend(
                {"Put": item.to_request_payload(table)} for item in self.items_to_put
            )
        if self.items_to_update:
            payload.extend(
                {"Update": item.to_request_payload(table)}
                for item in self.items_to_update
            )
        if self.items_to_delete:
            payload.extend(
                {"Delete": item.to_request_payload(table)}
                for item in self.items_to_delete
            )
        return payload

And in the client:

async def transact_write_items(
        self,
        table: TableName,
        request: TransactWriteItems,
        *
        request_token: Optional[str] = None,
    ):
        payload = {
            "ClientRequestToken": request_token,
            "TransactItems": request.to_request_payload(table=table),
        }
        response = await self.send_request(action="TransactWriteItems", payload=payload)
        return response

I think it would be better to have a return object of the operation and also think about ConditionCheck. But before thinking about that I just want to know if this design is something that can work for you.
What do you think?

[ojii]

I think that's quite close to what I had in mind. I don't think we need TransactWriteItems though, transact_write_items should just take an argument items which is a List[Union[Put, Update, Delete, ConditionCheck]].

The table_name needs to be moved from the method to those items since each item can target a different table (this also means this API will only be available on Client, not Table).

The method itself should guard against as many of the limitations of this API as we can easily do client side (minimum one item, maximum 25, ...)

[nicolaszein]

Nice @ojii! 😁

Good to know that we are on the right track.

So I will know move to open a PR and we can continuing discussing there. What do you think?

[ojii]

So I will know move to open a PR and we can continuing discussing there. What do you think?

sounds good. makes it easier to comment on specific lines/pieces of code too

[nicolaszein]

Thanks for your replies @ojii.

I opened the #132. If you could, please take a look. 😃

[ojii]

implemented in 22.8.