Extract Ldap as an EnvironmentExtender#22958
Conversation
Praveen2112
force-pushed
the
praveen/ldap_pt_cleanup
branch
2 times, most recently
from
27d4785 to
2768a9f
Compare
kokosing
left a comment
kokosing
left a comment
There was a problem hiding this comment.
% comment
It is nice to see composition, over inheritance
There was a problem hiding this comment.
Isn't this already exposed? I think it is something that should be exposed for coordinator as it is a common case. If not maybe it should land in Standard instead?
There was a problem hiding this comment.
It depends on the type of Coordinator - Only with for server with TLS 8443 is exposed.
There was a problem hiding this comment.
Standard exposes 8080 - But 8443 is special port so we could expose them on Environment specific.
There was a problem hiding this comment.
Maybe we should have Tls env extender?
...src/main/java/io/trino/tests/product/launcher/env/environment/EnvSinglenodeLdapInsecure.java
Outdated
Show resolved
Hide resolved
Praveen2112
force-pushed
the
praveen/ldap_pt_cleanup
branch
from
2768a9f to
8f6e012
Compare
There was a problem hiding this comment.
remove wrapping with ImmutableList.of(
There was a problem hiding this comment.
remove wrapping with ImmutableList.of(
There was a problem hiding this comment.
remove wrapping with ImmutableList.of(
There was a problem hiding this comment.
remove wrapping with ImmutableList.of(
...roduct-tests-launcher/src/main/java/io/trino/tests/product/launcher/env/common/OpenLdap.java
Outdated
Show resolved
Hide resolved
We are replacing AbstractEnvSinglenode with this dedicated Ldap environment
Praveen2112
force-pushed
the
praveen/ldap_pt_cleanup
branch
from
8f6e012 to
adddda0
Compare
|
@ssheikin Thanks for the review. AC |
| @@ -0,0 +1,7 @@ | |||
| password-authenticator.name=ldap | |||
There was a problem hiding this comment.
Where does this file come from? Looks like it did not exist previously?
There was a problem hiding this comment.
Previously it was referred from a common class now we have duplicated it specific to this class.
4 participants
We are replacing AbstractEnvSinglenode with this dedicated Ldap environment
Description
Additional context and related issues
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: