#20039 closed enhancement (implemented)
integrate `DescriptorIndexCollector` in a fully backward-compatible way
| Reported by: | iwakeh | Owned by: | iwakeh |
|---|---|---|---|
| Priority: | High | Milestone: | metrics-lib 1.5.0 |
| Component: | Metrics/Library | Version: | |
| Severity: | Normal | Keywords: | |
| Cc: | Actual Points: | ||
| Parent ID: | Points: | ||
| Reviewer: | Sponsor: |
Child Tickets
Change History (10)
comment:1 Changed 3 years ago by
| Owner: | changed from karsten to iwakeh |
|---|---|
| Status: | new → assigned |
comment:2 Changed 3 years ago by
Second change:
DescriptorIndexCollector#collectDescriptors will take either a full URL to an index.json variant or in case a base URL like https://x.y.z/a/b is given attempt to download index.json.gz from there, i.e. attempt to download https://x.y.z/a/b/index.json.gz.
comment:3 Changed 3 years ago by
Please review this branch containing the changes discussed in Berlin (comment:1 and comment:2).
The changes here and in #19934 made the *Node classes even smaller by transferring code to either DescriptorIndexDownloader here in metrics-lib or to CreateIndexJson in CollecTor.
This solution is not ideal
The four classes FileType, FileNode, DirectoryNode, and IndexNode are currently maintained in both collector and metrics-lib. Eventually, there should be only one place for them. It seems that the duplicate maintenance effort will most likely cause more work in the long run than extending metrics-lib's api by these four files.
comment:4 Changed 3 years ago by
| Status: | assigned → needs_review |
|---|
comment:5 Changed 3 years ago by
| Milestone: | metrics-lib 1.5.0 → metrics-lib 1.4.1 |
|---|---|
| Status: | needs_review → assigned |
I'll add the appropriate changes to the branch from comment:3, i.e. when there is only a base url download from <base-url>/index/index.json.
comment:6 Changed 3 years ago by
Please review the two commits on this branch. The first commit is just adding unused-imports-check to checkstyle.
comment:7 Changed 3 years ago by
| Priority: | Medium → High |
|---|---|
| Status: | assigned → needs_review |
set to high.
comment:9 Changed 3 years ago by
| Resolution: | → implemented |
|---|---|
| Status: | needs_review → closed |
All done, change released. The open question is addressed in #20405.
Closing.
Thanks!
comment:10 Changed 3 years ago by
| Milestone: | metrics-lib 1.4.1 → metrics-lib 1.5.0 |
|---|
The index.json functionality belongs to CollecTor; metrics-lib only keeps a minimal copy of the *Node classes to facilitate reading of index.json variants.
The master copy is located in CollecTor incl. the relevant tests.
Reasoning:
Best would be to issue an independent module of CollecTor to provide index.json* reading functionality. In order to reduce api maintenance this is not done and a simple copy is kept in metrics-lib to enable index.json* reading.