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

Golang bindings: third time is the charm? #1081

Closed

Conversation

vsoch
Copy link
Member

@vsoch vsoch commented Sep 28, 2023

Follow up from #1062.

I don't know how to do the interactive rebase so I did it manually, hopefully didn't mess something up.

@vsoch vsoch force-pushed the third-time-the-charm branch from c7f2fd6 to e1f5d98 Compare September 28, 2023 02:22
cmisale and others added 9 commits September 27, 2023 22:19
Go clients need to call REAPI CLI functions.

Add Go bindings for REAPI CLI for resource module
initialization, destruction, match allocate,
cancel, and other common functions.
Go clients need to call REAPI module functions.

Add Go bindings for REAPI module for resource module
initialization, destruction, match allocate,
cancel, and other common functions.
Problem: the REAPI module assumes the resource module
is already initialized but does not open a handle
to it.

Open a handle to make calls with a resource module
instance.
Problem: the Golang bindings need to be tested
as part of the Fluxion testsuite.

Add a main Go test to initialize the resource module
and output strings for comparison with expected
outputs.
Problem: the main test file must be
built as part of the `make check` tests.

Configure autotools to build main.go
for sharness tests.
Problem: the test matrix does not have
an image with Go installed.

Create a new Docker image which installs
Go and adds `go` to `PATH`. Add the image
and additional tests to the CI test matrix.
Problem: the Fluxion sharness tests need to test the
Golang bindings.

Add a basic sharness test and expected outputs.
Problem: Go bindings can be more friendly to Go developers
Solution: This includes docstring changes, the main module path
to be under flux-framework, returning Go error instead of int,
and updating tests to return nil (no error) instead of 0. The
module path is fixed from a development variant to a
flux-framework one. Finally, it fixes the Go bindings to
use a struct instead of passing around a ctx variable. Our
goal with these final changes to the Go module is to make
the code more friendly to future go developers.

Signed-off-by: vsoch <[email protected]>
Problem: the new cmake build system needs to compile go tests for bindings
Solution: use add_custom_command to do a build of the test.
Signed-off-by: vsoch <[email protected]>
@vsoch vsoch force-pushed the third-time-the-charm branch from e1f5d98 to 1d7635b Compare September 28, 2023 04:19
@codecov
Copy link

codecov bot commented Sep 28, 2023

Codecov Report

❗ No coverage uploaded for pull request base (master@4b16578). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head b59e864 differs from pull request most recent head 1d7635b. Consider uploading reports for the commit 1d7635b to get more accurate results

@@           Coverage Diff            @@
##             master   #1081   +/-   ##
========================================
  Coverage          ?   71.6%           
========================================
  Files             ?      89           
  Lines             ?   11569           
  Branches          ?       0           
========================================
  Hits              ?    8289           
  Misses            ?    3280           
  Partials          ?       0           

@vsoch
Copy link
Member Author

vsoch commented Sep 28, 2023

@milroy !!! @cmisale !!! 😍

🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏 📗 🟢 💚 🥗 🍵 🟩 🍏

Dear deity that I don't believe in. Or maybe just the light up dinosaur on my desk that I talk to sometimes. Please let this PR be good. I promise... well not much. Maybe I will share more avocados. Please let this be the third time is the charm, really really, this time! 🙏 -v

@vsoch
Copy link
Member Author

vsoch commented Sep 28, 2023

Closing here because we want to merge into the PR with the correct discussion, #1062, and mergify is having a hot minute looking at the equivalent commits.

@vsoch vsoch closed this Sep 28, 2023
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.

3 participants