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

python: Fix kz python binding errors #1537

Merged
merged 1 commit into from May 28, 2018

Conversation

Projects
None yet
5 participants
@dongahn
Copy link
Contributor

dongahn commented May 26, 2018

This PR contains one commit that fixes some issues in KZ Python binding.
I worked with @twrs to fix these issues so this is credited to him.

Fix two errors that prevented us from using Python kz binding
for @koning's testing.

  • A Python handle temporary object was garbaged collected
    before being passed to C kz function.
  • One of the variable was set to False when it should be set
    to None.

Resolve #1536.

python: Fix kz python binding errors
Done by @twrs.

Fix two errors that prevented us from using Python kz binding.
  - A Python handle temporary object was garbaged collected
    before being passed to C kz function.
  - One of the variable was set to False when it should be set
    to None.

@dongahn dongahn force-pushed the dongahn:kz_python branch from 676d808 to a573624 May 26, 2018

@dongahn

This comment has been minimized.

Copy link
Contributor Author

dongahn commented May 26, 2018

FYI -- while I was working on this, I accidentally created a branch in our upstream repo which I later removed. We may get some side effects because of this. My sincere apology.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented May 26, 2018

Codecov Report

Merging #1537 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #1537      +/-   ##
=========================================
- Coverage    78.8%   78.8%   -0.01%     
=========================================
  Files         164     164              
  Lines       30333   30333              
=========================================
- Hits        23905   23904       -1     
- Misses       6428    6429       +1
Impacted Files Coverage Δ
src/common/libflux/rpc.c 93.22% <0%> (-0.85%) ⬇️
src/common/libflux/response.c 83.73% <0%> (-0.82%) ⬇️
src/broker/overlay.c 73.81% <0%> (-0.32%) ⬇️
src/common/libflux/message.c 81.29% <0%> (ø) ⬆️
src/bindings/lua/flux-lua.c 82.19% <0%> (+0.08%) ⬆️
src/cmd/flux-module.c 85.36% <0%> (+0.3%) ⬆️
@coveralls

This comment has been minimized.

Copy link

coveralls commented May 26, 2018

Coverage Status

Coverage decreased (-0.003%) to 79.094% when pulling a573624 on dongahn:kz_python into 6fbffbd on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented May 28, 2018

I'll go ahead and merge since it sounds like @trws pre-approves.

@garlick garlick merged commit 8bbdc3b into flux-framework:master May 28, 2018

3 of 4 checks passed

codecov/project 78.8% (-0.01%) compared to 6fbffbd
Details
codecov/patch Coverage not affected when comparing 6fbffbd...a573624
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.003%) to 79.094%
Details
@trws

This comment has been minimized.

Copy link
Member

trws commented May 29, 2018

Thanks @dongahn, @garlick!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.