Opened 3 years ago

Closed 2 years ago

#20405 closed enhancement (worksforme)

create metrics-tools with all of the index.json processing code as first content

Reported by: iwakeh Owned by: karsten
Priority: Medium Milestone:
Component: Metrics/Library Version:
Severity: Normal Keywords:
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

cf. #20039 and #19934

Any better ideas to resolve this issue?

Child Tickets

Change History (6)

comment:1 Changed 3 years ago by iwakeh

Summary: all of the index.json processing code should be in metrics-libcreate metrics-tools with all of the index.json processing code as first content

Actually, as was discussed a while ago:

There should be another lib, metrics-tools (working title), that contains functionality on one hand needed by Metrics products and on the other hand not really in scope of any product.

The index-json functionality qualifies for being provided by metrics-tools as both CollecTor and metrics-lib depend on it, but is out of scope for either one. And, this would shield the actual json-processing lib used, e.g. a switch from gson to something better would be easier in future.

Changing the title to reflect the topic drift.

comment:2 Changed 3 years ago by karsten

Couple of thoughts:

  • We might consider metrics-lib as the library to facilitate data exchange between CollecTor and other applications/services processing CollecTor data.
  • As a result, we could grant CollecTor access to implementation packages of metrics-lib that applications/services should not touch. There's a tight coupling between CollecTor and metrics-lib anyway, so we wouldn't have to inform "those other CollecTor devs over there" to update their code, because we're deprecating a feature.
  • Before we start a new library, we should look at what functionality we're currently missing and whether that would fit into metrics-lib. Examples are date formatting and parsing, which could be solved by either introducing a new date class in metrics-lib instead of passing long values around, or it could be solved by providing a utilities class for processing those long values. Another example is fingerprint handling, where we'd provide methods for calculating the digest, for converting from hex to base64, and so on. Maybe we'll end up with a very short list for the new library that we decide it's not worth creating one; at the price of making metrics-lib more powerful. That being said, if something doesn't fit in, we shouldn't force it.

comment:3 Changed 3 years ago by iwakeh

Milestone: metrics-lib 1.5.0metrics-lib 2.0.0

comment:4 Changed 2 years ago by karsten

Status: newneeds_information

I gave this some more thoughts and am unconvinced that we need another library for index.json processing or similar code.

Let's consider index.json processing here, because we already have that code. The main purpose of extracting all code related to index.json production/consumption and putting it in a single place would be that we can change it in a single place quite easily. And we can test it more easily. Agreed, that would be useful.

But let's take a closer look at how much overlap there's between the code in CollecTor and in metrics-lib:

  • CollecTor converts a directory tree to Java instances, serializes those to JSON, and writes the resulting index.json string to disk.
  • metrics-lib downloads the index.json file from a remote server, deserializes it to Java instances, and downloads contained files.

The only code that could be shared here is the specification how Java instances are serialized to JSON and back. That's not really much code, maybe a few dozen lines of code. That's not nothing, but it's also hardly significant.

But my main concern is that we'd create yet another library that makes it yet a bit harder for new contributors to see the big picture. Maybe we'll need such a library in the future, but I don't see how we're there yet.

Changing status to needs_information, in case I missed something important here. But if not, I'd say let's close as wontfix.

comment:5 Changed 2 years ago by karsten

Milestone: metrics-lib 2.0.0

Regardless of whether or not we're creating a new library, this change seems unrelated to the metrics-lib 2.0.0 release, so I'm removing the milestone.

comment:6 Changed 2 years ago by iwakeh

Resolution: worksforme
Status: needs_informationclosed

I agree, all together there is not enough code/functionality to justify a new lib.
The shared code is pretty stable and changes to the JSON structure are unlikely in the near or mid-term future.

Closing.

Thanks!

Note: See TracTickets for help on using tickets.