Opened 3 years ago

Closed 3 years ago

Last modified 3 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:

Description

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 3 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.

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.

comment:2 Changed 3 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 3 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 3 years ago by iwakeh

Status: assignedneeds_review

comment:5 Changed 3 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 3 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 3 years ago by iwakeh

Priority: MediumHigh
Status: assignedneeds_review

set to high.

comment:8 Changed 3 years ago by karsten

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

comment:9 Changed 3 years ago by iwakeh

Resolution: implemented
Status: needs_reviewclosed

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

Thanks!

comment:10 Changed 3 years ago by iwakeh

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