Skip to content

NIFI-5875 Improve docs around the PriorityAttributePrioritizer - #3205

Closed
emilyselwood wants to merge 5 commits into
apache:masterfrom
emilyselwood:patch-2
Closed

NIFI-5875 Improve docs around the PriorityAttributePrioritizer#3205
emilyselwood wants to merge 5 commits into
apache:masterfrom
emilyselwood:patch-2

Conversation

@emilyselwood

Copy link
Copy Markdown
Contributor

Clear up the documentation around the PriorityAttributePrioritizer to make it clearer that if the attributes are both number the largest wins but if they are not numbers they sort the other way around.

I had to go and look at the code to work out what was going on after reading the existing documentation.

Thank you for submitting a contribution to Apache NiFi.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced
    in the commit message?

  • Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

  • Has your PR been rebased against the latest commit within the target branch (typically master)?

  • Is your initial contribution a single, squashed commit?

For documentation related changes:

  • Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

Clear up the documentation around the PriorityAttributePrioritizer to make it clearer that if the attributes are both number the largest wins but if they are not numbers they sort the other way around.

I had to go and look at the code to work out what was going on after reading the existing documentation.

@ijokarumawak ijokarumawak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wselwood Thanks for clarifying docs! Although I agree with the idea of adding more details, the proposed updated doc could be too specific for some users.

I'd suggest leaving the original sentence as it is, and add some notes for detailed behavior. Thoughts?

- *NewestFlowFileFirstPrioritizer*: Given two FlowFiles, the one that is newest in the dataflow will be processed first.
- *OldestFlowFileFirstPrioritizer*: Given two FlowFiles, the one that is oldest in the dataflow will be processed first. 'This is the default scheme that is used if no prioritizers are selected'.
- *PriorityAttributePrioritizer*: Given two FlowFiles that both have a "priority" attribute, the one that has the highest priority value will be processed first. Note that an UpdateAttribute processor should be used to add the "priority" attribute to the FlowFiles before they reach a connection that has this prioritizer set. Values for the "priority" attribute may be alphanumeric, where "a" is a higher priority than "z", and "1" is a higher priority than "9", for example.
- *PriorityAttributePrioritizer*: Given two FlowFiles, an attribute called “priority” will be extracted. If only one has that attribute it will go first. If the attributes can be parsed as a long, they will be ordered largest number first. For example 9 will come before 1 and 100 will be selected before 99. If only one of them parses as a long that one will be considered to have the higher priority. Otherwise, they will be ordered in alphanumeric order, where "a" is a higher priority than "z", and "1" is a higher priority than "9", for example.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the attributes can be parsed as a long, they will be ordered largest number first. For example 9 will come before 1 and 100 will be selected before 99

Is this true? As following test case shows, if 1 and 2 are compared, 1 goes first.
https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-standard-prioritizers/src/test/java/org/apache/nifi/prioritizer/PriorityAttributePrioritizerTest.java#L91

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouch. Good spot. The old docs got me spun around there. I'll update.

Simplify the main section of documentation for PriorityAttributePrioritzer and then add a notes section with more details about the ordering.
@ijokarumawak

Copy link
Copy Markdown
Member

@wselwood Thanks for the update, looks much better! For the even better readability, can we improve structure a little bit more organized? How about something like this?

Example output:

image


(As plain text)
PriorityAttributePrioritizer: Given two FlowFiles that both have a "priority" attribute, the one that has the highest priority value will be processed first.

  • Note that an UpdateAttribute processor should be used to add the "priority" attribute to the FlowFiles before they reach a connection that has this prioritizer set.
  • If only one has that attribute it will go first.
  • Values for the "priority" attribute may be alphanumeric, where "a" is a higher priority than "z", and "1" is a higher priority than "9", for example.
  • If "priority" attribute cannot be parsed as a long, unicode string ordering will be used. For example: "99" and "100" will be ordered so the flowfile with "99" comes first, but "A-99" and "A-100" will sort so the flowfile with "A-100" comes first.

@ijokarumawak

Copy link
Copy Markdown
Member

Looks perfect, +1 merging. Thanks @wselwood !

@asfgit asfgit closed this in 1a937b6 Dec 11, 2018
@emilyselwood
emilyselwood deleted the patch-2 branch December 11, 2018 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants