Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support for find #18

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: support for find #18

wants to merge 2 commits into from

Conversation

vsoch
Copy link
Member

@vsoch vsoch commented Feb 15, 2025

This is a WIP to add find to fluxion-go. Currently I am getting back what looks like an OK result (out) but the error code is 199, which does not make sense. B̶e̶f̶o̶r̶e̶ ̶I̶ ̶c̶o̶n̶t̶i̶n̶u̶e̶ ̶a̶d̶d̶i̶n̶g̶ ̶t̶e̶s̶t̶s̶ ̶I̶ ̶n̶e̶e̶d̶ ̶t̶o̶ ̶f̶i̶g̶u̶r̶e̶ ̶o̶u̶t̶ ̶w̶h̶y̶ ̶t̶h̶e̶ ̶e̶r̶r̶o̶r̶ ̶c̶o̶d̶e̶ ̶i̶s̶ ̶1̶9̶9̶.̶ ̶H̶e̶r̶e̶ ̶i̶s̶ ̶w̶h̶a̶t̶ ̶t̶h̶a̶t̶ ̶l̶o̶o̶k̶s̶ ̶l̶i̶k̶e̶ ̶-̶ ̶w̶h̶e̶n̶ ̶I̶ ̶c̶h̶a̶n̶g̶e̶ ̶t̶h̶e̶ ̶c̶r̶i̶t̶e̶r̶i̶a̶ ̶t̶o̶ ̶̶s̶t̶a̶t̶u̶s̶=̶d̶o̶w̶n̶̶ ̶i̶t̶ ̶r̶e̶t̶u̶r̶n̶s̶ ̶s̶u̶c̶c̶e̶s̶s̶f̶u̶l̶l̶y̶ ̶(̶n̶o̶ ̶e̶r̶r̶o̶r̶)̶ ̶b̶u̶t̶ ̶w̶i̶t̶h̶ ̶n̶o̶ ̶o̶u̶t̶.̶ ̶S̶o̶ ̶I̶ ̶w̶o̶n̶d̶e̶r̶ ̶i̶f̶ ̶i̶t̶'̶s̶ ̶r̶e̶l̶a̶t̶e̶d̶ ̶t̶o̶ ̶t̶h̶e̶ ̶i̶n̶p̶u̶t̶ ̶c̶r̶i̶t̶e̶r̶i̶a̶ ̶o̶r̶ ̶p̶a̶r̶s̶i̶n̶g̶ ̶o̶f̶ ̶o̶u̶t̶p̶u̶t̶

Update - figured it out, see below.

image

The error code of 199 is coming from inside of the fluxion find function! So it's not a weird conversion between types, etc.

$ cat /tmp/find.txt 
We are running find
We have output
Second condition
Calling json decref
Final error code is 199

OK, it's the writers->emit_json function.

$ cat /tmp/find.txt 
We are running find
Return code after traverser_find is 0
Return code after writers emit json is 199
We have output
Second condition
Calling json decref
Final error code is 199

Interesting, so check_array_sizes I think is just possibly returning the size? In which case the value that gets passed up isn't an error!

We are running jgf emit_json
Return code of check array sizes is 183
We are running jgf emit_json
Return code of check array sizes is 199

Note that the function is here and (I would guess) the reason it returns a higher positive value is because the return code is calculated as the sum of the two calls:

https://github.com/flux-framework/flux-sched/blob/da6156addab5ec127b36cdee03c5d1f3e458d363/resource/writers/match_writers.cpp#L388-L390

That seems kind of weird? Going to add some prints to verify. The function json_array_size is from jansson and does seem to return the number of elements.

Verifying that we are returning the sum of sizes:

Array size 1 is 92
Array size 2 is 91
We are going to return 183
We are running jgf emit_json
Return code of check array sizes is 183
Array size 1 is 100
Array size 2 is 99
We are going to return 199
We are running jgf emit_json
Return code of check array sizes is 199

If that is really supposed to be a return code, I think we should be doing the following in the function that calls it.

- if ((rc = check_array_sizes ()) <= 0)
-        goto ret;
+ rc = 0;

In the above, if the call is successful and returns a positive number, we reset it to 0. I think we could also just do:

    if (check_array_sizes () <= 0) {
        rc =  -1;
        goto ret;
   }

If there is something meaningful negative return value, we could capture it into another value (and set to rc if it's <=0. And then in the success case, rc will still be 0 from when it was originally set. I think for now I'm going to assume that anything greater than zero returned from find is a success. This might be "business as usual" for C++, but it did go against my expectation and I think we might either:

  1. Do some fix so the return code is a return code and not a count, or
  2. Update docstrings to make it clear what we are getting back (and how to parse).
  3. For both, rename variables to make it more clear what they represent.

I'll pick up on this tomorrow - can't wait to try the rest of find! It seems like the first way to peek into what fluxion is doing (or the state). 👀

This is a WIP to add find to fluxion-go. Currently I
am getting back what looks like an OK result (out)
but the error code is 199, which does not make sense.

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
This is due to a call to check_array_size
that returns an rc, but it is not actually
a return code, it is the sum of elements
in the array. The function can also return
a negative value (when things go wrong)
but it seems it can look like an error when
it goes right and returns a positive, nonzero
value. For the time being I am tweaking our
reapi error parser to call anything >=0
not an error (nil)

Signed-off-by: vsoch <vsoch@users.noreply.github.com>
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.

1 participant