simonbence commented on a change in pull request #4420:
URL: https://github.com/apache/nifi/pull/4420#discussion_r483547447
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/StatusHistoryUtil.java
##########
@@ -76,14 +76,14 @@ public static StatusDescriptorDTO
createStatusDescriptorDto(final MetricDescript
public static List<StatusDescriptorDTO> createFieldDescriptorDtos(final
Collection<MetricDescriptor<?>> metricDescriptors) {
final List<StatusDescriptorDTO> dtos = new ArrayList<>();
+ final Map<Integer, MetricDescriptor<?>> orderedDescriptors = new
HashMap<>();
- final Set<MetricDescriptor<?>> allDescriptors = new LinkedHashSet<>();
for (final MetricDescriptor<?> metricDescriptor : metricDescriptors) {
Review comment:
The order of the items counts, as it determines the order of the metrics
in the answer JSON, thus the order of the items in the UI.
The two-step mechanism was introduced to provide the ascending order
specified by the identifier of the metric. As the result StatusDescriptorDTO
does not contain this information, only its position carries the expected
ordering, I had to ask back to the original MetricDescriptor. The incoming
collection is not guaranteed to be ordered.
An other possible way would be to sort, as you mention, and not in the end
result but the input argument or more precisely on its copy. Copying and then
sorting that collection does not look much nicer in my perspective so if you do
not insist, I would keep it this way.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
