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

Tes 234 improve documentation #16

Merged

Conversation

kristianiliev1
Copy link
Contributor

@kristianiliev1 kristianiliev1 commented Oct 17, 2023

Description

Added initial description for all modules and main readme.md

Related Issues

https://ontotext.atlassian.net/browse/TES-234

Changes

Added All README.md files for Modules and main description

Screenshots (if applicable)

N/A

Checklist

  • I have tested these changes thoroughly.
  • My code follows the project's coding style.
  • I have added appropriate comments to my code, especially in complex areas.
  • All new and existing tests passed locally.

Reviewers:

@yaskoo @mihailradkov Please review this PR

Initial description add to README.md - How to use and what it is.
Initial add documentation for Backup Module
Initial description add for config module
Initial description add for DNS module
Initial description add for IAM module
Initial description add for load balancer module
Initial description add for user_data module
Initial description add for vm module
Add Hyperlinks to "Modules"
Add 1 more hyperlink
@kristianiliev1 kristianiliev1 marked this pull request as draft October 17, 2023 08:31
@kristianiliev1 kristianiliev1 marked this pull request as ready for review October 17, 2023 08:42
Copy link
Contributor Author

@kristianiliev1 kristianiliev1 left a comment

Choose a reason for hiding this comment

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

Test @

@yaskoo yaskoo requested review from mihailradkov and yaskoo October 17, 2023 10:12
README.md Outdated
@@ -1,15 +1,195 @@
# GraphDB AWS Terraform Module
# Terraform AWS GraphDB Module
Copy link
Contributor

Choose a reason for hiding this comment

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

IMO the previous sounded better.

README.md Outdated

TBD: Intro
This Terraform module allows you to provision an AWS GraphDB cluster within a Virtual Private Cloud (VPC) in AWS. The module provides a flexible way to configure the cluster and the associated VPC components.
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to limit the AWS reference, e.g.

This Terraform module allows you to provision a GraphDB cluster within a Virtual Private Cloud (VPC) in AWS. The module provides a flexible way to configure the cluster and the associated VPC components.

README.md Outdated

Check out the contributors guide [CONTRIBUTING.md](CONTRIBUTING.md).
This README provides more detailed information about the important variables and how to configure them for the Terraform AWS GraphDB module. Please ensure that you adapt the configuration to match your specific use case and requirements.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is lost here at the bottom, maybe move it somewhere in the configurations sections ?

README.md Outdated

TODO MORE

### Example Configuration
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be moved as an example under examples/ or simply refer to the one that we already have.

README.md Outdated
# Configure the required variables for the module.
```


Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid adding exessive/useless empty lines.

README.md Outdated

- `security_group_ids`: A list of security group IDs to associate with the instances for network security.

TODO MORE
Copy link
Contributor

Choose a reason for hiding this comment

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

Either finish this or submit a task.

README.md Outdated
## Prerequisites

Before you begin using this Terraform module, ensure you have the following prerequisites:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Before you begin using this Terraform module, ensure you have the following prerequisites:
Before you begin using this Terraform module, ensure you meet the following prerequisites:

README.md Show resolved Hide resolved
README.md Outdated
```shell
terraform apply
```

Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary empty lines.

README.md Show resolved Hide resolved
Changed the Documentation according Yasen's comments.

TES-234
README.md Outdated

- `instance_type`: The instance type for the GDB cluster nodes. This should match your performance and cost requirements.

- `cluster_size`: The number of instances in the cluster. Recommended is 3, 5 or 7 in order to have consensus according to the [RAFT algorithm](https://raft.github.io/).
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have a cluster_size variable?
Also, write Raft and not RAFT - it's not an abbreviation.

README.md Outdated

- `subnet_ids`: A list of subnet IDs in your VPC to place the Neptune instances. Ensure that these subnets have proper route tables and security groups configured.

- `security_group_ids`: A list of security group IDs to associate with the instances for network security.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't believe we have this variable as well.

README.md Outdated

ami_id = "GraphDB-AMI"

instance_type = "instance type"
Copy link
Contributor

Choose a reason for hiding this comment

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

Add real instance type or remove to use the default.

README.md Outdated

instance_type = "instance type"

graphdb_version = "10.4.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove this. Tomorrow this is going to be 10.4.1 and we don't want users deploying it after 2 years :D

README.md Show resolved Hide resolved
README.md Outdated
@@ -1,15 +1,117 @@
# GraphDB AWS Terraform Module

TBD: Intro
This Terraform module allows you to provision an GraphDB cluster within a Virtual Private Cloud (VPC). The module provides a flexible way to configure the cluster and the associated VPC components.
Copy link
Contributor

Choose a reason for hiding this comment

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

We could mention that this module "Implements the GraphDB reference architecture on AWS" - https://graphdb.ontotext.com/documentation/10.4/aws-deployment.html

Added a suggested link to Documentation by Yasen

TES-234
Copy link
Contributor

@yaskoo yaskoo left a comment

Choose a reason for hiding this comment

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

@mihailradkov do you have anything else we need to address here?

@yaskoo
Copy link
Contributor

yaskoo commented Oct 31, 2023

Update the branch before merging.

@kristianiliev1 kristianiliev1 merged commit 04ecfae into Ontotext-AD:main Oct 31, 2023
2 checks passed
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