Skip to content

Add a Builder for TlsConfig.#305

Merged
jkinkead merged 2 commits intoxjdr:masterfrom
jkinkead:tlsconfig_builder
Sep 24, 2018
Merged

Add a Builder for TlsConfig.#305
jkinkead merged 2 commits intoxjdr:masterfrom
jkinkead:tlsconfig_builder

Conversation

@jkinkead
Copy link
Copy Markdown
Collaborator

Let most values of TlsConfig come from non-config-file sources.

This ended up going through a few iterations, but it seems reasonable now.

I got rid of some helpers for creating TlsConfig out of paths in typesafe configs, because it felt like a weird abstraction (and was only used in unit tests). I'm happy to add them back in.

Let most values of TlsConfig come from non-config-file sources.
@jkinkead jkinkead requested a review from pdex September 14, 2018 22:46
@jkinkead
Copy link
Copy Markdown
Collaborator Author

Fixes #290 .

Copy link
Copy Markdown
Collaborator

@pdex pdex left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Collaborator

@manimaul manimaul left a comment

Choose a reason for hiding this comment

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

nice

Long sessionTimeout) {
// Validate that *all* parameters were set in the builder.
String errorMessage = "{} must be set in builder";
Preconditions.checkNotNull(useSsl, errorMessage, "useSsl");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i'd expect lombok to be able to make this not null requirement check with an annotation

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lombok.Builder is lacking in generated validation hooks, for that reason we've moved this sort of functionality over to google auto builder, we can refactor later.

@jkinkead jkinkead merged commit cab5185 into xjdr:master Sep 24, 2018
@jkinkead jkinkead deleted the tlsconfig_builder branch September 24, 2018 22:44
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.

3 participants