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

Support a directory as a valid location to include in the server's configuration doc changes #6872

Closed
rsherget opened this issue Sep 14, 2023 · 9 comments · Fixed by #6975
Closed
Assignees
Labels
23.0.0.10 peer reviewed technical reviewed An SME reviewed and approved the documentation from a technical perspective.
Milestone

Comments

@rsherget
Copy link

rsherget commented Sep 14, 2023

A directory can now be used in the includes element in the server.xml. The configuration files within the specified directory will all be processed and used by the server. It will not be done recursively; only the files within the directory will be used, not any files within any subdirectories. The files within the directory are processed alphabetically, the same way that files are processed within the configDropins folder (as specified here https://openliberty.io/docs/latest/reference/config/server-configuration-overview.html#server-xml).

Previous server.xml include

    <include location="./common/"/>

Previous server startup log

[ERROR   ] CWWKG0090E: The common/ configuration resource does not exist or cannot be read. 

The change added will add a check for directories. Instead of passing over the directory without utilizing it, we will now look at the files within the directory and add them to the server configuration. This will only check the child files of the specified directory, it will not go any deeper than that. If another directory is found within the specified directory, then it will be skipped to avoid any significant impact on startup time with a long recursive method. The files within the directory are processed in alphabetical order.

After changes server.xml include

    <include location="./common/"/>

After changes server startup log

[AUDIT   ] CWWKG0028A: Processing included configuration resource: /Users/rickyherget/libertyGit/open-liberty/dev/build.image/wlp/usr/servers/com.ibm.ws.config.include.directory/common/a.xml
[AUDIT   ] CWWKG0028A: Processing included configuration resource: /Users/rickyherget/libertyGit/open-liberty/dev/build.image/wlp/usr/servers/com.ibm.ws.config.include.directory/common/b.xml
[AUDIT   ] CWWKG0028A: Processing included configuration resource: /Users/rickyherget/libertyGit/open-liberty/dev/build.image/wlp/usr/servers/com.ibm.ws.config.include.directory/common/c.xml

I was thinking these two docs:

Which release will the runtime updates target? The Open Liberty docs team will aim to get the docs updates published in sync with the runtime updates.

230010

@dmuelle
Copy link
Member

dmuelle commented Sep 14, 2023

Hi @rsherget - is there an development epic associated with this change?

https://openliberty.io/docs/latest/reference/config/include.html
is autogenerated from the metatype files and any changes there are made by development, not docs team. But the other page we would update to explain the behavior change. Is there a development issue for this work? Thanks

@rsherget
Copy link
Author

Yes, the epic associated with this change is found here OpenLiberty/open-liberty#25149.

@dmuelle dmuelle added this to the 23.0.0.10 milestone Sep 15, 2023
@dmuelle
Copy link
Member

dmuelle commented Sep 15, 2023

Thanks @rsherget, I've added this issue to our 23.0.0.10 milestone. Although epic is marked No Design Approved, which generally indicates no ID requirement, I've added the ID Required- Trivial label to the epic to indicate we are making minor changes to align with the update. We will review at our upcoming scrum and let you know if we have any questions.

@dmuelle dmuelle assigned dmuelle and ramkumar-k-9286 and unassigned dmuelle Sep 19, 2023
ramkumar-k-9286 added a commit that referenced this issue Oct 4, 2023
Directory Handling-Includes Element

#6872
@ramkumar-k-9286
Copy link
Contributor

Hi Ricky @rsherget

Made the suggested correction to the documentation:

Draft link: https://docs-draft-openlibertyio.mqj6zf7jocq.us-south.codeengine.appdomain.cloud/docs/latest/reference/config/server-configuration-overview.html#include-processing

Please review the same and add the Developer Reviewed label if you are happy with the changes

Regards,
Ramkumar

@rsherget
Copy link
Author

rsherget commented Oct 4, 2023

Looks good to me, thanks!

ramkumar-k-9286 added a commit that referenced this issue Oct 6, 2023
Directory Handling-Includes Element-3

#6872
@ramkumar-k-9286
Copy link
Contributor

Hi Ricky @rsherget

I had to make some corrections based on comments received from my doc lead.

The updated doc link: https://docs-draft-openlibertyio.mqj6zf7jocq.us-south.codeengine.appdomain.cloud/docs/latest/reference/config/server-configuration-overview.html#include-processing

Also, you mentioned that 2 documents need to be updated. One of them has been updated.
https://openliberty.io/docs/latest/reference/config/include.html -( Needs to be updated from the development side).

The file can be accessed in the following location:
open-liberty/dev/com.ibm.ws.config/resources/com/ibm/ws/config/internal/resources/SchemaData.nlsprops

File Name is: SchemaData.nlsprops

Line no: 28 needs to be updated.

Existing line: config.internal.metatype.includeType.attribute.location.documentation=Specifies the resource location. This can be a file path or a URI for a remote resource.

can be replaced with the following:

config.internal.metatype.includeType.attribute.location.documentation=Specifies the resource location. The value can be a file path, a directory, or a URI for a remote resource.

Regards,
Ramkumar

CC @dmuelle

@dmuelle
Copy link
Member

dmuelle commented Oct 6, 2023

Hi Ram- a couple suggestions

  • At the beginning of this section, link the first mention of the include element to the autogen by using the config macro. Also, clean the sentence up a but while we're at it :)

In addition to the default locations, additional configuration files can be brought in by using the include element.
--->
In addition to the default locations, you can bring in additional configuration files by using the config:include[] element.

  • clarify the new sentence, use active voice, and use terminology that is consistent with the rest of the section (process rather than read/use)

The server reads and uses the files directly within the specified directory, ignoring any subdirectories and the files within them to avoid any significant impact on startup time. The files within the directory are also processed in alphabetical order.

--->

When you specify a directory as the location value, the server processes only the files that are directly within the specified directory and ignores any subdirectories. The server processes the files within the directory in alphabetical order.

  • for consistency with the other examples, enclose the include in a server element.
<server>
    <include location="./common/"/>
</server>

ramkumar-k-9286 added a commit that referenced this issue Oct 9, 2023
Directory Handling-Includes Element-4

#6872
@ramkumar-k-9286
Copy link
Contributor

@dmuelle
Copy link
Member

dmuelle commented Oct 9, 2023

LGTM, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
23.0.0.10 peer reviewed technical reviewed An SME reviewed and approved the documentation from a technical perspective.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants