spotify / heroic

The Heroic Time Series Database
https://spotify.github.io/heroic/
Apache License 2.0
848 stars 109 forks source link

Optionally write metadata resource identifiers as tags #725

Closed adsail closed 3 years ago

adsail commented 3 years ago

Introduce a config variable to write metadata resource identifiers to elasticsearch in addition to tags. By default, RIs will continue to not be written as tags. If configured as true, RIs are appended to the SortedMap of tags and written as tags to elasticsearch.

codecov[bot] commented 3 years ago

Codecov Report

Merging #725 (2c09d09) into master (5234ed9) will increase coverage by 0.42%. The diff coverage is 95.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #725      +/-   ##
============================================
+ Coverage     54.40%   54.83%   +0.42%     
- Complexity     3052     3084      +32     
============================================
  Files           735      736       +1     
  Lines         19868    20081     +213     
  Branches       1304     1309       +5     
============================================
+ Hits          10810    11012     +202     
- Misses         8591     8600       +9     
- Partials        467      469       +2     
Impacted Files Coverage Δ Complexity Δ
...c/test/AbstractMetadataBackendIndexResourceIT.java 94.47% <94.47%> (ø) 29.00 <29.00> (?)
...ata/elasticsearch/ElasticsearchMetadataModule.java 72.35% <100.00%> (+1.17%) 6.00 <0.00> (ø)
...roic/metadata/elasticsearch/MetadataBackendKV.java 81.53% <100.00%> (+0.66%) 49.00 <2.00> (+3.00)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 5234ed9...2c09d09. Read the comment docs.

lmuhlha commented 3 years ago

👍 nice job. would be good to make that one bit of code more generic so it can be called to in a tag or resource situation, as peter suggested. otherwise LGTM