Skip to content

Conversation

@barrysteyn
Copy link

@barrysteyn barrysteyn commented Nov 18, 2019

Table names must always be deterministic - if a name is defined, then it MUST not be random since it is probably designed to be used in application code.

This fixes logic in #4 and addresses #6. @dodgeblaster Please look at this over. I have done the following:

  1. If name is defined, then there is no random id added onto the name
  2. Simplified logic greatly in setTableName, and added comment (now easy to understand what's happening).
  3. Removed nameInput state variable (less state means less errors).

In summary: this PR accomplishes what was set out to achieve in #4, but neatens up the logic and allows for a deterministic (i.e. non-random) table name if name is defined.

I can't stress how important it is that this PR be addressed.

Please @eahefnawy can you look at it right away. The logic is right now broken. Why? Because if I define a table name in this component, I can't refer to it in code since the name becomes random. This becomes a bigger problem when you have multiple instances of the same component (i.e. a prod and staging component), and so one needs the name to be deterministic so that you don't write special logic in each case to get to the table name.

Lastly, @danielcondemarin also needs this logic for the global table component.

@barrysteyn
Copy link
Author

@eahefnawy Also, please can you merge #9 in as well. Preferably, merge this first, and then #9

@dodgeblaster
Copy link
Contributor

@barrysteyn You can use the generated table name in your application code. This component outputs the name of the generated table, which you can use as an input to another component. Example:

name: example-project dynamo: component: '@serverless/aws-dynamodb' inputs: name: table-name attributeDefinitions: - AttributeName: PK AttributeType: S - AttributeName: SK AttributeType: S keySchema: - AttributeName: PK KeyType: HASH - AttributeName: SK KeyType: RANGE region: us-east-1 function: component: ./code inputs: dbName: ${dynamo.name} dbArn: ${dynamo.arn}
@barrysteyn
Copy link
Author

barrysteyn commented Nov 18, 2019

@barrysteyn You can use the generated table name in your application code. This component outputs the name of the generated table, which you can use as an input to another component.

What you have described is different. You are describing using component inputs in other components. However, I am talking about the actual code that a user would use the component to deploy and run. Sure, if that component is aware of the table name, it could then pass it to the app code. However this is not very generic.

A good example is the amazing next.js component: It does not care about dynamodb (nor should it), since a user should be able to use whatever storage they desire (e.g. MongoDB). And so that component would never inject the name of the table into the app code.

Copy link
Member

@eahefnawy eahefnawy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the super late response here @barrysteyn ... I think this change is good to go, I was just nervous of making changes to the table name as it's usually destructive, but it seems that you're handling it pretty well.

LGTM

@eahefnawy eahefnawy merged commit 2bf8af1 into serverless-components:master Dec 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

3 participants