You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/12/09 15:09:07 UTC
[GitHub] [maven-resolver] cstamas opened a new pull request, #228: [MRESOLVER-305] Handle blocked state at connector level
cstamas opened a new pull request, #228:
URL: https://github.com/apache/maven-resolver/pull/228
As blocked is really about block remote access. Locally cached things should not be affected, but current solution did not cover all execution paths.
---
https://issues.apache.org/jira/browse/MRESOLVER-305
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #228: [MRESOLVER-305] Handle blocked state at connector level
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #228:
URL: https://github.com/apache/maven-resolver/pull/228#discussion_r1044588589
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryConnectorProvider.java:
##########
@@ -109,6 +109,20 @@ public RepositoryConnector newRepositoryConnector( RepositorySystemSession sessi
throws NoRepositoryConnectorException
{
requireNonNull( repository, "remote repository cannot be null" );
+
+ if ( repository.isBlocked() )
+ {
+ if ( repository.getMirroredRepositories().isEmpty() )
+ {
+ throw new NoRepositoryConnectorException( repository, "Blocked repository: " + repository );
Review Comment:
if you look from where this block was _removed_, it is in same try-catch where this method is invoked (and throws this exception). This move basically prevents any "remote access" to blocked repositories, not just for one case like originally. But yes, I agree, we do need test, I thought to reuse existing ones (are there any?)
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] kwin commented on a diff in pull request #228: [MRESOLVER-305] Handle blocked state at connector level
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #228:
URL: https://github.com/apache/maven-resolver/pull/228#discussion_r1044594871
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryConnectorProvider.java:
##########
@@ -109,6 +109,20 @@ public RepositoryConnector newRepositoryConnector( RepositorySystemSession sessi
throws NoRepositoryConnectorException
{
requireNonNull( repository, "remote repository cannot be null" );
+
+ if ( repository.isBlocked() )
+ {
+ if ( repository.getMirroredRepositories().isEmpty() )
+ {
+ throw new NoRepositoryConnectorException( repository, "Blocked repository: " + repository );
Review Comment:
Ok, sorry, now I see the related catch block in https://github.com/apache/maven-resolver/blob/752756f123cde40249f3d87490d3c2daa957127e/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java#L584 and
https://github.com/apache/maven-resolver/blob/a8c0b56a614b1759f0384ab3326c301d3a7221a7/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultMetadataResolver.java#L631 and indeed that should work just as expected.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #228: [MRESOLVER-305] Handle blocked state at connector level
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #228:
URL: https://github.com/apache/maven-resolver/pull/228#discussion_r1044588589
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryConnectorProvider.java:
##########
@@ -109,6 +109,20 @@ public RepositoryConnector newRepositoryConnector( RepositorySystemSession sessi
throws NoRepositoryConnectorException
{
requireNonNull( repository, "remote repository cannot be null" );
+
+ if ( repository.isBlocked() )
+ {
+ if ( repository.getMirroredRepositories().isEmpty() )
+ {
+ throw new NoRepositoryConnectorException( repository, "Blocked repository: " + repository );
Review Comment:
if you look from where this block was _removed_, it is in same try-catch where this method is invoked (and throws this exception). This basically prevents any "remote access" to blocked repositories.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] kwin commented on a diff in pull request #228: [MRESOLVER-305] Handle blocked state at connector level
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #228:
URL: https://github.com/apache/maven-resolver/pull/228#discussion_r1044590603
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryConnectorProvider.java:
##########
@@ -109,6 +109,20 @@ public RepositoryConnector newRepositoryConnector( RepositorySystemSession sessi
throws NoRepositoryConnectorException
{
requireNonNull( repository, "remote repository cannot be null" );
+
+ if ( repository.isBlocked() )
+ {
+ if ( repository.getMirroredRepositories().isEmpty() )
+ {
+ throw new NoRepositoryConnectorException( repository, "Blocked repository: " + repository );
Review Comment:
But if I have 1 blocked remote repo and one not-blocked one I expect the latter to be used instead (if possible). Resolving should only fail if the requested artifact/metadata could not be delivered from any non-blocked remote repo.
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryConnectorProvider.java:
##########
@@ -109,6 +109,20 @@ public RepositoryConnector newRepositoryConnector( RepositorySystemSession sessi
throws NoRepositoryConnectorException
{
requireNonNull( repository, "remote repository cannot be null" );
+
+ if ( repository.isBlocked() )
+ {
+ if ( repository.getMirroredRepositories().isEmpty() )
+ {
+ throw new NoRepositoryConnectorException( repository, "Blocked repository: " + repository );
Review Comment:
But if I have one blocked remote repo and one not-blocked one I expect the latter to be used instead (if possible). Resolving should only fail if the requested artifact/metadata could not be delivered from any non-blocked remote repo.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #228: [MRESOLVER-305] Handle blocked state at connector level
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #228:
URL: https://github.com/apache/maven-resolver/pull/228#discussion_r1044593073
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryConnectorProvider.java:
##########
@@ -109,6 +109,20 @@ public RepositoryConnector newRepositoryConnector( RepositorySystemSession sessi
throws NoRepositoryConnectorException
{
requireNonNull( repository, "remote repository cannot be null" );
+
+ if ( repository.isBlocked() )
+ {
+ if ( repository.getMirroredRepositories().isEmpty() )
+ {
+ throw new NoRepositoryConnectorException( repository, "Blocked repository: " + repository );
Review Comment:
Yes, as connector provider provides connector _for one given remote repo_ (is param on method), so it will happily provide connector instance for other non-blocked remote repositories....
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] kwin commented on a diff in pull request #228: [MRESOLVER-305] Handle blocked state at connector level
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #228:
URL: https://github.com/apache/maven-resolver/pull/228#discussion_r1044577421
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryConnectorProvider.java:
##########
@@ -109,6 +109,20 @@ public RepositoryConnector newRepositoryConnector( RepositorySystemSession sessi
throws NoRepositoryConnectorException
{
requireNonNull( repository, "remote repository cannot be null" );
+
+ if ( repository.isBlocked() )
+ {
+ if ( repository.getMirroredRepositories().isEmpty() )
+ {
+ throw new NoRepositoryConnectorException( repository, "Blocked repository: " + repository );
Review Comment:
we should probably have some tests, because to me it is not clear where this exception is caught and logged. Does it prevent other remote repos from being tried or not?
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas commented on a diff in pull request #228: [MRESOLVER-305] Handle blocked state at connector level
Posted by GitBox <gi...@apache.org>.
cstamas commented on code in PR #228:
URL: https://github.com/apache/maven-resolver/pull/228#discussion_r1044618718
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryConnectorProvider.java:
##########
@@ -109,6 +109,20 @@ public RepositoryConnector newRepositoryConnector( RepositorySystemSession sessi
throws NoRepositoryConnectorException
{
requireNonNull( repository, "remote repository cannot be null" );
+
+ if ( repository.isBlocked() )
+ {
+ if ( repository.getMirroredRepositories().isEmpty() )
+ {
+ throw new NoRepositoryConnectorException( repository, "Blocked repository: " + repository );
Review Comment:
Yes, in short: resolver will refuse to provide any connector for blocked RemoteRepository instances. Everywhere.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] kwin commented on a diff in pull request #228: [MRESOLVER-305] Handle blocked state at connector level
Posted by GitBox <gi...@apache.org>.
kwin commented on code in PR #228:
URL: https://github.com/apache/maven-resolver/pull/228#discussion_r1044594871
##########
maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRepositoryConnectorProvider.java:
##########
@@ -109,6 +109,20 @@ public RepositoryConnector newRepositoryConnector( RepositorySystemSession sessi
throws NoRepositoryConnectorException
{
requireNonNull( repository, "remote repository cannot be null" );
+
+ if ( repository.isBlocked() )
+ {
+ if ( repository.getMirroredRepositories().isEmpty() )
+ {
+ throw new NoRepositoryConnectorException( repository, "Blocked repository: " + repository );
Review Comment:
Ok, sorry, now I see the related catch block in https://github.com/apache/maven-resolver/blob/752756f123cde40249f3d87490d3c2daa957127e/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultArtifactResolver.java#L584 and indeed that should work just as expected.
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [maven-resolver] cstamas merged pull request #228: [MRESOLVER-305] Handle blocked state at connector level
Posted by GitBox <gi...@apache.org>.
cstamas merged PR #228:
URL: https://github.com/apache/maven-resolver/pull/228
--
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: issues-unsubscribe@maven.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org