[ 
https://issues.apache.org/jira/browse/BEAM-10838?focusedWorklogId=720959&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-720959
 ]

ASF GitHub Bot logged work on BEAM-10838:
-----------------------------------------

                Author: ASF GitHub Bot
            Created on: 04/Feb/22 15:53
            Start Date: 04/Feb/22 15:53
    Worklog Time Spent: 10m 
      Work Description: egalpin commented on pull request #16690:
URL: https://github.com/apache/beam/pull/16690#issuecomment-1030114682


   I apologize for the slowness on my review, trying to carve out time.  I've 
taken a scan so far and the changes look good generally. With respect to tests, 
I agree that this is a tricky one to draw the lines on.  We don't want to be 
testing every external module we might use (Ex. we don't directly test the 
`RestClient` methods on their own).  Keeping that in mind I think we should be 
sure to test that the code surrounding SSLContext produces the desired 
behaviour in the pipeline and place no effort on testing SSLContext directly. I 
think it would be sufficient to show that any singular code path making use of 
a non-null SslConfiguration can be used to run a pipeline to read or write data.
   
   My other questions are around backward compatibility/the update story for 
users with existing pipelines that may need to update to this version. I 
haven't yet satisfied myself with an answer to the question of whether or not 
there are compatibility issues.
   
   A few things you could do in the mean time @clandry94 would be ensuring that 
the tests and `spotless` linter both pass:
   
   - You can run `./gradlew spotlessApply` from the top level of the beam 
project directory on your machine to apply linting changes automatically
   - You can run `./gradlew 
:sdks:java:io:elasticsearch-tests:elasticsearch-tests-7:test` to run unit tests 
locally, as it appears as though the new test is failing:  
https://ci-beam.apache.org/job/beam_PreCommit_Java_Commit/20847/testReport/org.apache.beam.sdk.io.elasticsearch/ElasticsearchIOTest/testSslConfiguration/
   - Feel free to ignore the `Java_Examples_Dataflow_Java17` failure as this is 
expected and unrelated.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 720959)
    Time Spent: 1h 20m  (was: 1h 10m)

> Add a withSSLContext builder method to ElasticsearchIO 
> -------------------------------------------------------
>
>                 Key: BEAM-10838
>                 URL: https://issues.apache.org/jira/browse/BEAM-10838
>             Project: Beam
>          Issue Type: New Feature
>          Components: io-java-elasticsearch
>            Reporter: Conor Landry
>            Assignee: Conor Landry
>            Priority: P2
>          Time Spent: 1h 20m
>  Remaining Estimate: 0h
>
> Currently, the ElasticsearchIO transforms have only one way to securely 
> read/write Elasticsearch by using the withKeystorePath builder method and 
> providing the location of a keystore containing a client key in jks format.
> This is a bit limiting, especially for Elasticsearch users not depending on 
> shield to secure their clusters. I'd like to propose the addition of the 
> builder method withSSLContext(SSLContext sslContext, which delegates to 
> `httpClientBuilder.setSSLContext`.
> If this is (y), I can start working on it.



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

Reply via email to