dskit icon indicating copy to clipboard operation
dskit copied to clipboard

Make `flagext` pkg consistent across different projects

Open kavirajk opened this issue 4 years ago • 5 comments

Currently Loki uses, its own version of flagext package

Should we also bring this to dskit?

One of the rationale from my side personally is, it will make entitlement system validation simpler. Having it in single place. e.g: I can implement custom JSON unmarshalling for these types in one place.

kavirajk avatar Sep 07 '21 09:09 kavirajk

You mean to port improvements from Loki's flagext package to dskit?

aknuds1 avatar Sep 07 '21 09:09 aknuds1

Yes!. that's right.

Would be great if all GL projects (like Loki, Tempo, Cortex) uses same flagext package.

kavirajk avatar Sep 07 '21 09:09 kavirajk

@kavirajk The flagext from Cortex was already ported to dskit. What specifically is the one in dskit missing compared to loki's copy?

I'm closing this since Loki has already been updated to use the version from DSKit: https://github.com/grafana/loki/pull/4225

56quarters avatar Sep 07 '21 13:09 56quarters

@56quarters Loki still have its custom version of flagext (already mentioned in the description) and its still using it. My point is we should bring that also into dskit. And Loki should use only flagext from dskit!.

kavirajk avatar Sep 07 '21 14:09 kavirajk

@56quarters Loki still have its custom version of flagext (already mentioned in the description) and its still using it. My point is we should bring that also into dskit. And Loki should use only flagext from dskit!.

I think it depends on what we wanna port. Is the data type specific to Loki? Then I would keep it in Loki. Is the data type generic (ex. human bytes) then we can move it to dskit if can benefit other projects too.

pracucci avatar Sep 14 '21 07:09 pracucci