feat(internal): add Maven native dependency client #1207
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1207 +/- ##
==========================================
- Coverage 65.96% 65.09% -0.87%
==========================================
Files 168 170 +2
Lines 14152 14400 +248
==========================================
+ Hits 9335 9374 +39
- Misses 4299 4506 +207
- Partials 518 520 +2 ☔ View full report in Codecov by Sentry. |
cuixq
changed the title
| insights pb.InsightsClient | ||
| } | ||
|
|
||
| func NewMavenRegistryClient(registry string) (*MavenRegistryClient, error) { |
There was a problem hiding this comment.
For future private registry support, this will probably need to support multiple registries at a time, right?
|
|
||
| return &MavenRegistryClient{ | ||
| api: datasource.NewMavenRegistryAPIClient(registry), | ||
| insights: pb.NewInsightsClient(conn), |
There was a problem hiding this comment.
Since the InsightsClient is only seems to be used for PreFetch, it might be ok to just create it there, instead of storing it in the MavenRegistryClient
There was a problem hiding this comment.
the only difference is that we are not able to return the error of making the insights client but this seems fine - the worst result is that we are not not going to pre-fetch anything.
There was a problem hiding this comment.
actually this may be better that failure on insights client won't block us of making a registry client.
| return gob.NewDecoder(f).Decode(&c.api) | ||
| } | ||
|
|
||
| func (c *MavenRegistryClient) PreFetch(ctx context.Context, imports []resolve.RequirementVersion, manifestPath string) { |
There was a problem hiding this comment.
For my own reference: I actually dislike needing to make PreFetch an interface method of each DependencyClient, since it's almost the same for each client, and also because it needs the deps.dev API to work.
After this is merged, I want to look into making PreFetch a standalone function that calls every resolve.Client method for caching, and use something like singleflight to avoid making identical network requests multiple times.
There was a problem hiding this comment.
yeah I think this is a good idea, I am also thinking about making some cache interface with setter and getter so we don't need to lock and unlock every time.
…t` interface and prevent repeated datasource network calls (#1224) What I mentioned in #1207 (comment) Make `PreFetch` a standalone function that takes in a client that uses every `DependencyClient` method call. Since the underlying datasources tend to use the same request for multiple methods, I've made a `requestCache` type that uses logic based on the [singleflight](https://cs.opensource.google/go/x/sync/+/refs/tags/v0.8.0:singleflight/singleflight.go;l=91) package to prevent the same requests being made multiple times. I've simplified it a bit by skipping the bespoke handling of panics / `runtime.Goexit`.
#1045
This PR adds a dependency client fetching from Maven Central repository: