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

Add description about placeholders of database.properties #1498

Merged
merged 7 commits into from
Feb 16, 2024

Conversation

kota2and3kan
Copy link
Contributor

Description

This PR adds the description about placeholders in the database.properties file.

We added placeholder support in the database.properties file in PR #770.

There is a Javadoc, which describes this feature, but there is no description in Scalar Docs site.

So, I added the description of the placeholder feature in the document.

Please take a look!

Related issues and/or PRs

Changes made

  • Update docs/configurations.md.

Checklist

  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation to reflect the changes.
  • Any remaining open issues linked to this PR are documented and up-to-date (Jira, GitHub, etc.).
  • Tests (unit, integration, etc.) have been added for the changes.
  • My changes generate no new warnings.
  • Any dependent changes in other PRs have been merged and published.

Additional notes (optional)

N/A

Release notes

N/A

@kota2and3kan kota2and3kan requested review from josh-wong and a team February 14, 2024 03:00
@kota2and3kan kota2and3kan self-assigned this Feb 14, 2024
@kota2and3kan kota2and3kan requested review from komamitsu, brfrn169, feeblefakie and Torch3333 and removed request for a team February 14, 2024 03:00
Copy link
Contributor

@komamitsu komamitsu 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!

@@ -186,6 +186,10 @@ The following are additional configurations available for ScalarDB:
| `scalar.db.default_namespace_name` | The given namespace name will be used by operations that do not already specify a namespace. | |
| `scalar.db.system_namespace_name` | The given namespace name will be used by ScalarDB internally. | `scalardb` |

## Use placeholders

You can use placeholders in the values, and they are replaced with environment variables (${env:<ENVIRONMENT_VARIABLE_NAME>}) or system properties (${sys:<SYSTEM_PROPERTY_NAME>}). You can also specify default values in placeholders like ${sys:<SYSTEM_PROPERTY_NAME>:-<DEFAULT_VALUE>}.
Copy link
Contributor

Choose a reason for hiding this comment

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

environment variables (${env:<ENVIRONMENT_VARIABLE_NAME>})

How about syntax-highlighting it as follows?

environment variables (`${env:<ENVIRONMENT_VARIABLE_NAME>}`)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see. Looks good!
I will apply it. Thank you for your suggestion!

Copy link
Contributor

@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!

Copy link
Member

@josh-wong josh-wong left a comment

Choose a reason for hiding this comment

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

I've added some suggestions. PTAL!

docs/configurations.md Outdated Show resolved Hide resolved
docs/configurations.md Outdated Show resolved Hide resolved
docs/configurations.md Outdated Show resolved Hide resolved
kota2and3kan and others added 2 commits February 15, 2024 12:18
Co-authored-by: Josh Wong <joshua.wong@scalar-labs.com>
Copy link
Member

@josh-wong josh-wong left a comment

Choose a reason for hiding this comment

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

I left two additional comments on parts that I had mistakenly looked over. Other than that, LGTM. Thank you!🙇🏻‍♂️

docs/configurations.md Outdated Show resolved Hide resolved
docs/configurations.md Outdated Show resolved Hide resolved
Co-authored-by: Josh Wong <joshua.wong@scalar-labs.com>
@@ -258,3 +262,14 @@ scalar.db.contact_points=indirect:<SCALARDB_CLUSTER_CONTACT_POINT>
```

For details about client configurations, see the ScalarDB Cluster [client configurations (redirects to the Enterprise docs site)](https://scalardb.scalar-labs.com/docs/latest/scalardb-cluster/developer-guide-for-scalardb-cluster-with-java-api/#client-configurations).

### Configuration example #3 - Placeholder usage
Copy link
Collaborator

@brfrn169 brfrn169 Feb 16, 2024

Choose a reason for hiding this comment

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

I think we can move this example into the ## Placeholder usage part. I think this Configuration example section is more like for the actual/real settings. So if this example is only for the placeholder usage, I think we can put it in the ## Placeholder usage part. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. Indeed, this example does not include the actual configuration (minimum configuration) to access the backend DB or ScalarDB Cluster.
I agree with you. I will move this example into the ## Placeholder usage section. Thank you!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated it in 910c269.

Copy link
Collaborator

@brfrn169 brfrn169 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!

@brfrn169 brfrn169 merged commit 0c23ecb into master Feb 16, 2024
23 checks passed
@brfrn169 brfrn169 deleted the update-docs-apache-commons-text branch February 16, 2024 02:31
feeblefakie pushed a commit that referenced this pull request Feb 16, 2024
Co-authored-by: Josh Wong <joshua.wong@scalar-labs.com>
Co-authored-by: Toshihiro Suzuki <brfrn169@gmail.com>
feeblefakie pushed a commit that referenced this pull request Feb 16, 2024
Co-authored-by: Josh Wong <joshua.wong@scalar-labs.com>
Co-authored-by: Toshihiro Suzuki <brfrn169@gmail.com>
feeblefakie pushed a commit that referenced this pull request Feb 16, 2024
Co-authored-by: Josh Wong <joshua.wong@scalar-labs.com>
Co-authored-by: Toshihiro Suzuki <brfrn169@gmail.com>
feeblefakie pushed a commit that referenced this pull request Feb 16, 2024
Co-authored-by: Josh Wong <joshua.wong@scalar-labs.com>
Co-authored-by: Toshihiro Suzuki <brfrn169@gmail.com>
feeblefakie pushed a commit that referenced this pull request Feb 16, 2024
Co-authored-by: Josh Wong <joshua.wong@scalar-labs.com>
Co-authored-by: Toshihiro Suzuki <brfrn169@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants