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

Openapi custom context root 1.x #1524

Merged
merged 6 commits into from
Mar 17, 2020
Merged

Openapi custom context root 1.x #1524

merged 6 commits into from
Mar 17, 2020

Conversation

tjquinno
Copy link
Member

Resolves #1350 for 1.x. Very similar to the 2.x changes for the same issue.

@tjquinno tjquinno added the 1.x Issues for 1.x version branch label Mar 16, 2020
@tjquinno tjquinno self-assigned this Mar 16, 2020
* @return updated builder instance
*/
public Builder helidonConfig(Config config) {
config.get(CONFIG_PREFIX + ".web-context")
Copy link
Member

Choose a reason for hiding this comment

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

No prefix should be used, config should be on openapi node already.
Method should be called config

Copy link
Member Author

Choose a reason for hiding this comment

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

This method, helidonConfig, has been present on SEOpenAPISupportBuilder in SE OpenAPI since we first created it, as has the expectation that the Config node passed in is the top-level one, within which openapi might exist.

Because we want to support Helidon config in MP also, this method is migrating to the superclass OpenAPISupport. So while it might appear that this is a new method, it is not a new part of the contract between SE OpenAPI and SE apps.

Changing either the method name or the expectation of which config node is passed - in 1.x anyway - would be a backward-incompatible change.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can make the incompatible change in the 2.x PR to align OpenAPI with the other components that have similar config support.

Copy link
Member Author

Choose a reason for hiding this comment

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

For 1.x, we can add the config method as you suggest -- with a Config argument that should be the openapi node -- while keeping the existing helidonConfig method to preserve backward compatibility and mark helidonConfig as deprecated.

Copy link
Member

Choose a reason for hiding this comment

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

yes please

…precated; add parallel config method which accepts the openapi config node; related changes
@tjquinno tjquinno added this to In Progress in Backlog Mar 17, 2020
@tjquinno tjquinno merged commit 7a86a55 into helidon-io:helidon-1.x Mar 17, 2020
Backlog automation moved this from In Progress to Closed Mar 17, 2020
@tjquinno tjquinno deleted the openapi-custom-context-root-1.x branch March 17, 2020 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x Issues for 1.x version branch
Projects
Backlog
  
Closed
Development

Successfully merging this pull request may close these issues.

None yet

2 participants