Skip to content

Conversation

ymorimo
Copy link
Contributor

@ymorimo ymorimo commented Nov 1, 2020

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

The following works have been done to remove dependencies on Cassandra from the universal/scalardl module.
(But there is no option to switch the database other than Cassandra yet.)

  • cassandra_username and cassandra_password have been renamed to database_username and database_password.
  • replication_factor has been renamed to cassandra_replication_factor since it's going to be a Cassandra-specific option.
  • database_contact_points has been added to universal/scalardl module, but cassandra-lb is still hardcoded in the aws/azure modules.
  • database_contact_port is going to be needed but has not been added yet.

The base branch of this PR is dynamodb. I plan to open subsequent PRs toward this branch.

@ymorimo ymorimo requested review from feeblefakie and tei-k November 1, 2020 13:36
@ymorimo ymorimo self-assigned this Nov 1, 2020
@feeblefakie
Copy link
Collaborator

Oh, you've done this in the story !!!
The story was supposed to be for implementing DynamoDB deployment in scalar-terraform only
and I am planning to make the ledger part cloud agnostic in https://scalar-labs.atlassian.net/browse/DLT-7496.
Anyway, it's done so it's fine. 👍

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!

scalardl_image_name = var.scalardl_image_name
scalardl_image_tag = var.scalardl_image_tag
internal_domain = var.internal_domain
database_contact_points = "cassandra-lb.${var.internal_domain}" # TODO: add to variables
Copy link
Collaborator

Choose a reason for hiding this comment

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

If it's going to be done in another PR, can you create a story not to forget?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the variable is going to be added later.
Created as a subtask https://scalar-labs.atlassian.net/browse/DLT-7605
For database_contact_port variable: https://scalar-labs.atlassian.net/browse/DLT-7524

@tei-k
Copy link
Contributor

tei-k commented Nov 6, 2020

Overall looking good. 👍

Also need to update README.
https://github.com/scalar-labs/scalar-terraform/blob/master/modules/aws/scalardl/cluster/README.md
https://github.com/scalar-labs/scalar-terraform/blob/master/modules/azure/scalardl/cluster/README.md

@ymorimo
Copy link
Contributor Author

ymorimo commented Nov 6, 2020

@tei-k Thanks! Updated two README files.

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!

@feeblefakie feeblefakie merged commit 4c0576b into dynamodb Nov 6, 2020
@feeblefakie feeblefakie deleted the remove-cassandra-deps-from-universal-scalardl branch November 6, 2020 06:05
@ymorimo ymorimo mentioned this pull request Nov 30, 2020
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