-
Notifications
You must be signed in to change notification settings - Fork 121
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
S3 maven artifacts support #275
base: master
Are you sure you want to change the base?
Conversation
|
||
This code was inspired by the [aether examples](https://github.com/eclipse/aether-demo/blob/322fa556494335faaf3ad3b7dbe8f89aaaf6222d/aether-demo-snippets/src/main/java/org/eclipse/aether/examples/GetDependencyTree.java) for walking maven dependencies. | ||
|
||
S3 Handler was based on [frugalmechanic/fm-sbt-s3-resolver](https://github.com/frugalmechanic/fm-sbt-s3-resolver/). |
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'm open to merging something like this, but I'd like to add some documentation on how this code is working and how users can interact with it. This is a nice link, but we need more here, I think.
code match { | ||
case 200 => "OK" | ||
case 404 => "Not Found" | ||
case _ => "DUMMY" |
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'd rather this be something like: unexpected code: $code
@@ -0,0 +1,102 @@ | |||
package com.github.johnynek.bazel_deps | |||
|
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.
is this file mostly original code, or largely copied? If copied can you provide some git permalinks here to where it came from to help others.
Also, is there any way we could use the compiled version of the original code rather than adding as much code here?
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.
Larged copied, I was doing this just to unblock me, so I didn't really think it through a lot. That being said, I'm really open to make any modifications to help others to use it.
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.
My first thinking of bringing the code here was to avoid any reference to sbt
int the code. But maybe I could try to use as a dependency and just override what is needed by this project. I'll see if it works out, if not I'll add links to github and do a better documentation.
So, the more I think about it, the more this seems like an issue for coursier. Have you thought of opening an issue there? |
Yeah, this was my first thought but then I found this coursier/coursier#164 and decided to do it here. But maybe it wasn't a good call, I'll read more and if that's the case, I'll open an issue there. |
About using the original code: It was designed Probably a dead end :( |
import com.amazonaws.services.securitytoken.{AWSSecurityTokenService, AWSSecurityTokenServiceClient} | ||
import com.amazonaws.services.securitytoken.model.{AssumeRoleRequest, AssumeRoleResult} | ||
import org.apache.ivy.util.url.URLHandler | ||
import org.apache.ivy.util.{CopyProgressEvent, CopyProgressListener, Message} |
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.
ivy dependencies feel like a large beast to be bringing in, what are they giving us here?
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'm not sure, but this solution didn't consider coursier first class, so I'm assuming this is something we can get rid of. I couldn't work on this for quite some time, but now I'm back and I'll probably look into this soon.
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.
That would be great, seems like a very useful addition to have
Not sure if useful, but I had the same problem with
sbt
. When trying to resolve maven artificats from a private s3 path, theCouriserResolver
would fail. Forsbt
this problems was solved by https://github.com/frugalmechanic/fm-sbt-s3-resolverI pretty much hacked its code into
CoursierResolver
to unblock myself into usingbazel-deps
. I decided to share in case anyone bumps into the same problem. I'm not sure if this is the way to go, but I'll be happy to do any modifications if this is useful.