[PLUGIN-1732] - Successfactors Proxy Implementation #33
Conversation
Shubhangi-cs
force-pushed
the
feature/proxyAddition
branch
3 times, most recently
from
58b3ee5 to
e2d61b6
Compare
src/main/java/io/cdap/plugin/successfactors/source/transport/SuccessFactorsTransporter.java
Show resolved
Hide resolved
|
|
||
| private final String username; | ||
| private final String password; | ||
| private SuccessFactorsConnectorConfig config; |
There was a problem hiding this comment.
NIT : add empty line after private static final variables
| @@ -136,7 +136,16 @@ private Schema getOutputSchema(FailureCollector failureCollector) { | |||
| } catch (TransportException te) { | |||
| String errorMsg = ExceptionParser.buildTransportError(te); | |||
| errorMsg = ResourceConstants.ERR_ODATA_SERVICE_CALL.getMsgForKeyWithCode(errorMsg); | |||
| failureCollector.addFailure(errorMsg, null).withConfigProperty(SuccessFactorsPluginConfig.BASE_URL); | |||
| if ((errorMsg.contains("proxy") || errorMsg.contains("Connection reset"))) { | |||
There was a problem hiding this comment.
Is there a documentation that maps these KEY words to the given properties ?
Connection reset , Failed to connect can be very generic error messages and may not be related to the configs...
There was a problem hiding this comment.
As tested the errors thrown in these scenarios are always same and to highlight the errors on the particular fields these keywords needs to be used.
There was a problem hiding this comment.
That might be true,
but these key words may occur for other failures NOT related to proxy.
Like network failure or something.
In such cases, these suggestion might mislead a user in debugging in wrong direction.
There was a problem hiding this comment.
If there is no pre-defined error message for proxy related stuff, then Please surface the whole error.
It's not correct to assume error message, as these can change in future unless it's mentioned by their official doc.
| throws MalformedURLException, TransportException { | ||
| OkHttpClient.Builder builder = getConfiguredClient(); | ||
|
|
||
| if (proxyUrl != null && !proxyUrl.isEmpty()) { |
There was a problem hiding this comment.
I meant here sorry
use
!Strings.isNullOrEmpty(proxyUrl)
from
import com.google.common.base.Strings;
Shubhangi-cs
force-pushed
the
feature/proxyAddition
branch
from
e8d837d to
483ccc1
Compare
Shubhangi-cs
force-pushed
the
feature/proxyAddition
branch
from
fe407a3 to
cda12dc
Compare
Proxy Configuration :
Proxy URL: Users can enter the URL of the proxy server. This URL should contain the protocol (e.g., http or https), the proxy server's address, and the port number (e.g., http://proxy.example.com:8080).
Username (Optional): If the proxy server requires authentication, users can provide the username.
Password (Optional): If the proxy server requires authentication, users can supply the password.
This enhancement will enable users to set up and use Proxy settings in the Successfactors plugin, allowing them to route their requests through a proxy server when communicating with external endpoints.
https://cdap.atlassian.net/browse/PLUGIN-1732