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

identifyException accidentally externalized as unusable top level config element #17591

Closed
njr-11 opened this issue Jun 18, 2021 · 0 comments · Fixed by #17901
Closed

identifyException accidentally externalized as unusable top level config element #17591

njr-11 opened this issue Jun 18, 2021 · 0 comments · Fixed by #17901
Assignees
Labels
design-issue release bug This bug is present in a released version of Open Liberty release:21009 team:Zombie Apocalypse

Comments

@njr-11
Copy link
Contributor

njr-11 commented Jun 18, 2021

identifyException was intended to be available only as a nested element of dataSource,

<dataSource jndiName="jdbc/ds1" jdbcDriverRef="mydriver">
  <identifyException sqlState="08000" as="StaleConnection"/>
  <identifyException sqlState="08004" as="StaleConnection"/>
  <properties .../>
</dataSource>

When I went to promote this nested element from nonship to beta, I discovered that due to the way in which the metatype was written, the developer had accidentally exposed identifyException as an unusable top level element, which made it into Open Liberty doc. See https://openliberty.io/docs/21.0.0.1/reference/config/identifyException.html

This means that a user could currently write the following as valid top level config, which would be a no-op because it doesn't apply to anything:

<identifyException sqlState="08000" as="StaleConnection"/>

Options:

  1. Remove identifyException as top level config and add an exclude to the MetaTypeValidator. This shouldn't cause any real errors because the top level element never did anything in the first place, although schema-based config editors would now start noticing it as bad config, which in a way is helpful because it would inform users that their top level identifyException doesn't do anything.
  2. Leave identifyException as useless top level config, which the server will continue to ignore. The usability problem of having a useless configuration element will remain.
  3. Allow identifyException as top level config and come up with a way to make it useful by giving dataSource an identifyExceptionRef attribute. This complicates the configuration somewhat, but at least has a valid use case behind it. It would unfortunately force us to switch away from the flattened config optimization for identifyException.

Preferred option 1 was approved by the POC.

@njr-11 njr-11 self-assigned this Jun 18, 2021
@cbridgha cbridgha added this to Backlog in Design Issues Jun 29, 2021
@njr-11 njr-11 changed the title identifyException accidentally externalized as unusable top level config element [July 20] identifyException accidentally externalized as unusable top level config element Jun 30, 2021
@njr-11 njr-11 changed the title [July 20] identifyException accidentally externalized as unusable top level config element identifyException accidentally externalized as unusable top level config element Jun 30, 2021
@njr-11 njr-11 added the release bug This bug is present in a released version of Open Liberty label Jul 19, 2021
njr-11 added a commit to njr-11/open-liberty that referenced this issue Jul 19, 2021
@tevans78 tevans78 moved this from Backlog to Implementing in Design Issues Jul 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design-issue release bug This bug is present in a released version of Open Liberty release:21009 team:Zombie Apocalypse
Projects
Design Issues
Implementing
Development

Successfully merging a pull request may close this issue.

2 participants