#140 fix ssl problems #153
Conversation
| * Unless required by applicable law or agreed to in writing, software distributed under the License | ||
| * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express | ||
| * or implied. See the License for the specific language governing permissions and limitations under | ||
| * the License. | ||
| */ | ||
| package com.cognifide.aet.job.common.collectors.source; | ||
|
|
||
| import com.cognifide.aet.communication.api.metadata.CollectorStepResult; |
There was a problem hiding this comment.
The new imports order is good according to the Google styleguide:
https://google.github.io/styleguide/javaguide.html#s3.3-import-statements
however we have it different order in other files...
maybe we could use current convention and discuss what to do with not so google order of imports in future?
I mean: it might be better to have it consistent through all of files. What do you think?
There was a problem hiding this comment.
I guess we should finally have all imports consistent with formatter we recommend. The problem here is that Google changed formatting approach in one of previous formatters version.
I revert the change in imports order in files of this pull request and let's discuss what to to with it.
Thanks for noticing that :)
| content = (byte[]) future.get(timeoutValue, TimeUnit.MILLISECONDS); | ||
| } catch (TimeoutException | InterruptedException | ExecutionException e) { | ||
| return httpRequestExecutor.executeGetRequest(properties.getUrl(), timeoutValue).getContent(); | ||
| } catch (IOException e) { | ||
| throw new ProcessingException(e.getMessage(), e); |
There was a problem hiding this comment.
Maybe we could add error LOG with exception message and requested URL?
There was a problem hiding this comment.
This is already logged in CollectorDispatcherImpl in the catch block.
| @@ -71,7 +71,7 @@ | |||
| @Before | |||
| public void setUp() throws Exception { | |||
| when(wrapper.getWebDriver()).thenReturn(webDriver); | |||
| when(wrapper.getHttpRequestBuilder()).thenReturn(builder); | |||
| when(wrapper.getHttpRequestBuilder()).thenReturn(requestExecutor); | |||
There was a problem hiding this comment.
What do you think about also changing the method name from getHttpRequestBuilder() to getHttpRequestExecutor() ?
There was a problem hiding this comment.
Thanks, I missed that.
| */ | ||
| package com.cognifide.aet.worker.drivers; | ||
|
|
||
| import com.cognifide.aet.job.api.collector.HttpRequestExecutor; |
There was a problem hiding this comment.
Maybe we could stay with current, not so correct convention for imports order for now? (see another comment about imports order)
| @@ -54,9 +54,9 @@ public JsErrorLog apply(JavaScriptError input) { | |||
|
|
|||
| private final ProxyServerWrapper proxyServer; | |||
|
|
|||
| private final HttpRequestBuilder builder; | |||
| private final HttpRequestExecutor builder; | |||
There was a problem hiding this comment.
Could we rename the field from builder to executor ?
|
Thank you for the work @Skejven! I am still working on the support for new Karaf version, but the work goes slow.. |
malaskowski
deleted the
bugfix/#140-fix-ssl-problems-with-source-collector
branch
Ignore certificates when collecting source from the page.
I hereby agree to the terms of the AET Contributor License Agreement.