Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#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:


Adapt the alpha implementation from release 1.4.0, ticket #19791.

See comments 10 to 12 in #19791 for background.

Child Tickets

Change History (10)

comment:1 Changed 4 years ago by iwakeh

Owner: changed from karsten to iwakeh
Status: newassigned

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.

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.

comment:2 Changed 4 years ago by iwakeh

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 4 years ago by iwakeh

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 4 years ago by iwakeh

Status: assignedneeds_review

comment:5 Changed 4 years ago by iwakeh

Milestone: metrics-lib 1.5.0metrics-lib 1.4.1
Status: needs_reviewassigned

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 4 years ago by iwakeh

Please review the two commits on this branch. The first commit is just adding unused-imports-check to checkstyle.

comment:7 Changed 4 years ago by iwakeh

Priority: MediumHigh
Status: assignedneeds_review

set to high.

comment:8 Changed 4 years ago by karsten

Merged. Please close if this concludes the ticket. Thanks!

comment:9 Changed 4 years ago by iwakeh

Resolution: implemented
Status: needs_reviewclosed

All done, change released. The open question is addressed in #20405.


comment:10 Changed 4 years ago by iwakeh

Milestone: metrics-lib 1.4.1metrics-lib 1.5.0
Note: See TracTickets for help on using tickets.