search

dmwm / WMArchive

Workload Archive project
4 stars 13 forks source link

add worker node in wmarchive #350

Closed ticoann closed 6 years ago

ticoann commented 6 years ago

related https://github.com/dmwm/WMCore/pull/8470

ticoann commented 6 years ago

@vkuznet Valentin, please review

vkuznet commented 6 years ago

Seangchan, may be it's worth to put all metrics we discussed in https://github.com/dmwm/WMCore/pull/8304/ thread even if we'll not fill them out the schema will contain them already for future placement. How about to put wn_info as a dict and add attributes over there?

ticoann commented 6 years ago

Hi Valentin, I updated patch as you suggested. However, my impression was these parameters are not the concrete list. See comments from Antonio, Alan, https://github.com/dmwm/WMCore/issues/8468. That is one of the reason we decide not to include the matrix in wmagent yet. If you still sure these parameters will be used you can merge this patch. Otherwise, I can revert the patch (only include wn_name)

vkuznet commented 6 years ago

Seangchan, thanks for pointing this out. I think in this case we should revert to wn_node. Moreover, I think (I need to double check) that adding a complex data structure (such as dict or list) may break backward compatibility of the schema. Therefore it would be simpler to add just key:values into existing structure as you did before. I need time to perform tests with Avro/WMArchive schema to verify about adding new dict/lists. The trivial data-types, will have be assigned null values. For instance, adding wn_node:"string" will always have null for old data. But if we add dict/list I'm not sure that old schema(s) will allow that.

ticoann commented 6 years ago

Hi Valentin, I revert back the patch.