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

DynamoDB support #244

Merged
merged 30 commits into from
Dec 14, 2020
Merged

DynamoDB support #244

merged 30 commits into from
Dec 14, 2020

Conversation

ymorimo
Copy link
Contributor

@ymorimo ymorimo commented Nov 30, 2020

https://scalar-labs.atlassian.net/browse/DLT-7415

A PR to merge PRs already merged to the dynamodb branch (#232, #238, #237, and #242) to the master.

ymorimo and others added 22 commits October 31, 2020 23:57
…niversal-scalardl

Remove Cassandra dependencies from universal/scalardl module
Add `database` and `database_contact_port` variables to the universal/scalardl module
The DynamoDB resion should be specified in `database_contact_points` instead
since scalardb uses contact_points as a region.
…-image

Add trigger for schema_loader_image
…rsal-scalardl

Add database options to aws/azure/universal scalardl modules
…hema-loader

Upgrade scalar-ledger and scalardl-schema-loader to support DynamoDB
@ymorimo ymorimo requested review from feeblefakie and tei-k November 30, 2020 07:28
@ymorimo ymorimo self-assigned this Nov 30, 2020
@ymorimo ymorimo marked this pull request as draft November 30, 2020 07:40
@ymorimo
Copy link
Contributor Author

ymorimo commented Nov 30, 2020

I will work on fixing the failing GitHub Actions test.

@ymorimo
Copy link
Contributor Author

ymorimo commented Dec 3, 2020

#246 should fix the test.

@ymorimo
Copy link
Contributor Author

ymorimo commented Dec 7, 2020

Merged master to make checks pass

@ymorimo ymorimo marked this pull request as ready for review December 7, 2020 06:57
feeblefakie
feeblefakie previously approved these changes Dec 8, 2020
Copy link
Collaborator

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

LGTM! (left one question)

"%s %s",
"docker run --rm ${var.schema_loader_image} -h ${var.database_contact_points} -u ${var.database_username} -p ${var.database_password}",
var.database == "cassandra" ? "--cassandra -P ${var.database_contact_port} -n NetworkTopologyStrategy -R ${var.cassandra_replication_factor}" :
var.database == "dynamo" ? "--dynamo --region ${var.database_contact_points}" :
Copy link
Collaborator

Choose a reason for hiding this comment

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

We are working on adding Cosmos DB support in a later PR, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, only the schema-loading chart in scalar-k8s can load the schema to Cosmos DB, and I don't think we have any plans to support Cosmos DB by scalar-terraform for now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you just saying we don't have plans for Cosmos DB support in scalar-terraform, but it can be done easily like DynamoDB? Or it is difficult to do it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, we focused on k8s to support Cosmos DB so far. I think it's going to be easy to do it in scalar-terraform as well.

@ymorimo
Copy link
Contributor Author

ymorimo commented Dec 11, 2020

Working on resolving the conflicts.

This was referenced Dec 11, 2020
Copy link
Contributor

@tei-k tei-k left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

Copy link
Collaborator

@feeblefakie feeblefakie left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you!

@feeblefakie feeblefakie merged commit dcdb215 into master Dec 14, 2020
@feeblefakie feeblefakie deleted the dynamodb branch December 14, 2020 00:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants