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

FISH-9198 Add Microprofile Config TOML Support #6988

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

kalinchan
Copy link
Member

@kalinchan kalinchan commented Oct 1, 2024

Description

Resolves #6822

This feature introduces changes to the Admin Console and new AsAdmin Commands.

By default the ordinal for TOML is 95.

The properties will only be updated when first initialised or if the last modified time has been changed.

When the TOML file contains a nested array you can retrieve the value by specifying the index e.g. [1]

Admin Console

(Configurations -> server-config -> Microprofile -> Config -> TOML)
image

AsAdmin Commands

  • get-toml-config-source-configuration: Returns the path to the TOML file and the maximum recursion depth (to prevent potential stackoverflow)
  • set-toml-config-source-configuration: path parameter to specify location of toml file, can be relative to current domain. depth parameter to specify the depth of recursion.

Important Info

Blockers

n/a

Testing

New tests

none

Testing Performed

Specified a TOML file with various different tables and arrays and deployed a simple application that prints out all properties, the ordinal and specific values.

Testing Environment

Apache Maven 3.9.8 (36645f6c9b5079805ea5009217e36f2cffd34256)
Maven home: /Users/kalinchan/Library/apache-maven-3.9.8
Java version: 11.0.23, vendor: Azul Systems, Inc., runtime: /Users/kalinchan/.sdkman/candidates/java/11.0.23-zulu/zulu-11.jdk/Contents/Home
Default locale: en_GB, platform encoding: UTF-8
OS name: "mac os x", version: "14.6.1", arch: "aarch64", family: "mac"

Documentation

to come...

Notes for Reviewers

tomlj had to be repackaged due to not being an OSGi bundle and it also has a depedency on Antlr4

Had to implement the ordinal delegation for configsourceproxy.

Removed default recursion depth as it does not appear in admin console or when running admin command.

@kalinchan kalinchan force-pushed the FISH-9198 branch 2 times, most recently from 3537a1a to d8327e7 Compare October 1, 2024 14:28
@luiseufrasio luiseufrasio self-requested a review October 4, 2024 10:30
Copy link
Contributor

@luiseufrasio luiseufrasio left a comment

Choose a reason for hiding this comment

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

When the file does not exists on Windows it returns:

asadmin set-toml-configuration --path C:\config.tom --depth 10

Picked up JAVA_TOOL_OPTIONS: -Duser.language=en -Duser.country=US -Dfile.encoding=UTF-8
remote failure: java.nio.file.InvalidPathException: Illegal char <:> at index 105: C:\Users\luise\git\Payara\appserver\distributions\payara\target\stage\payara6\glassfish\domains\domain1\C:\config.tom
Illegal char <:> at index 105: C:\Users\luise\git\Payara\appserver\distributions\payara\target\stage\payara6\glassfish\domains\domain1\C:\config.tom
Command set-toml-configuration failed.

core/pom.xml Outdated Show resolved Hide resolved
Copy link
Member

@Pandrex247 Pandrex247 left a comment

Choose a reason for hiding this comment

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

I would also question whether the TOML config source should exist under the nucleus package - would it not be better homed alongside the other extensions?

Copy link
Contributor

@luiseufrasio luiseufrasio left a comment

Choose a reason for hiding this comment

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

LGTM!

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.

Enhancement: TOML Config Source for MicroProfile Config /FISH-9198
3 participants