New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proxy connection implementation #1706
base: master
Are you sure you want to change the base?
Conversation
DeepCode's analysis on #d00d85 found:
💬 This comment has been generated by the DeepCode bot, installed by the owner of the repository. The DeepCode bot protects your repository by detecting and commenting on security vulnerabilities or other critical issues. |
@strongbox/core-developers : For implementing SOCKS proxy we have to register ConnectionSocketFactory in One approach is separate |
@@ -1,5 +1,7 @@ | |||
package org.carlspring.strongbox.client; | |||
|
|||
import static org.assertj.core.api.Assertions.assertThat; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can safely revert this change. It just re-orders the import and it's not according to our convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
@@ -1,5 +1,7 @@ | |||
package org.carlspring.strongbox.service.impl; | |||
|
|||
import static org.assertj.core.api.Assertions.assertThat; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can safely revert this change. It just re-orders the import and it's not according to our convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
||
ProxyServerConfiguration getProxyServerConfiguration() | ||
throws IllegalAccessException, | ||
InvocationTargetException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cosmetic: Could you please set your IDE to align exceptions? (I've seen this elsewhere across the pull request in methods as well). Thanks! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@carlspring : Pls share the format, I am using the same template available on strongbox documentation page.
@@ -24,15 +25,15 @@ | |||
private static final Logger logger = LoggerFactory.getLogger(HttpGetRemoteRepositoryCheckStrategy.class); | |||
|
|||
@Inject | |||
private ProxyRepositoryConnectionPoolConfigurationService proxyRepositoryConnectionPoolConfigurationService; | |||
private ProxyRepositoryConnectionConfigurationService proxyRepositoryConnectionConfigurationService; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for my better understanding: why is this no lnger using the connection pool service configuration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ProxyRepositoryConnectionConfigurationService
is a wrapper over ProxyRepositoryConnectionPoolConfigurationService
, so both are doing same thing.
Pull Request Description
This pull request closes #1677
Acceptance Test
mvn clean install -Dintegration.tests
still works.mvn spring-boot:run
in thestrongbox-web-core
still starts up the application correctly.strongbox-distribution
from azip
ortar.gz
still works.strongbox-web-integration-tests
still run properly.Questions
Does this pull request break backward compatibility?
Does this pull request require other pull requests to be merged first?
Does this require an update of the documentation?