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

wreck: add flux-wreck cancel command #1365

Merged
merged 5 commits into from Mar 17, 2018

Conversation

Projects
None yet
5 participants
@grondo
Copy link
Contributor

grondo commented Mar 16, 2018

This adds a flux wreck cancel command in support of #1359.

Along the way I was hitting #1364 in travis so had to add a workaround for that here as well.

The actual implementation of flux wreck cancel is very simple and the majority of the important work will be on the sched side.

grondo added some commits Mar 16, 2018

cmd/flux-wreck: add cancel subcommand
Add a `flux wreck cancel` command to send sched.cancel
request for wreck jobid.
t/t2000-wreck: add sanity test for flux-wreck cancel
Test that flux-wreck cancel fails in the expected way when
the sched module is not loaded.
lua/posix.lua: use newer clock_gettime if available
If posix.time table exists in the luaposix library, then
assign posix.time.clock_gettime to posix.clock_gettime.
O/w, fallback to default deprecated version of clock_gettime.

This is a workaround for a bug in luaposix 34.0.4.

Fixes #1364
lua: always load posix module as flux.posix
In order to allow the flux/posix.lua wrapper to work around issues
in the luaposix bindings, ensure all flux commands and scripts
load posix as `flux.posix`.

@grondo grondo force-pushed the grondo:wreck-cancel branch from 2c358ee to f532324 Mar 16, 2018

@coveralls

This comment has been minimized.

Copy link

coveralls commented Mar 16, 2018

Coverage Status

Coverage decreased (-0.03%) to 78.813% when pulling f532324 on grondo:wreck-cancel into cee1139 on flux-framework:master.

@garlick

This comment has been minimized.

Copy link
Member

garlick commented Mar 16, 2018

Restarted a builder that appeared to hang in the cron tests.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Mar 16, 2018

Codecov Report

Merging #1365 into master will increase coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1365      +/-   ##
==========================================
+ Coverage   78.52%   78.59%   +0.07%     
==========================================
  Files         162      162              
  Lines       29741    29741              
==========================================
+ Hits        23355    23376      +21     
+ Misses       6386     6365      -21
Impacted Files Coverage Δ
src/modules/connector-local/local.c 74.38% <0%> (-1.44%) ⬇️
src/common/libutil/base64.c 95.07% <0%> (-0.71%) ⬇️
src/modules/kvs/kvs.c 65.21% <0%> (+0.16%) ⬆️
src/common/libflux/message.c 81.48% <0%> (+0.23%) ⬆️
src/common/libflux/reactor.c 93.73% <0%> (+0.28%) ⬆️
src/broker/overlay.c 74.45% <0%> (+0.31%) ⬆️
src/common/libflux/msg_handler.c 87.36% <0%> (+0.72%) ⬆️
src/connectors/local/local.c 88.14% <0%> (+0.74%) ⬆️
src/common/libflux/handle.c 84.4% <0%> (+0.74%) ⬆️
src/common/libflux/rpc.c 94.21% <0%> (+0.82%) ⬆️
... and 5 more
@garlick

This comment has been minimized.

Copy link
Member

garlick commented Mar 17, 2018

Pressing the button!

@garlick garlick merged commit 0448c22 into flux-framework:master Mar 17, 2018

4 checks passed

codecov/patch Coverage not affected when comparing cee1139...f532324
Details
codecov/project 78.59% (+0.07%) compared to cee1139
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.03%) to 78.813%
Details
usage = "JOBID",
handler = function (self, arg)
local id = check_jobid_arg (self, arg[1])
local resp, err = f:rpc ("sched.cancel", { jobid = id })

This comment has been minimized.

@dongahn

dongahn Mar 17, 2018

Contributor

@grondo: I know this PR is closed, but I thought I should ask a question as I am testing this.

I'm using flux_request_unpack () to unpack this RPC request within flux-sched, and I initially used { s:I } to fetch the value of id. But this this led to a protocol error.

Then, I realized what's needed is ={ s:s } for conversion.

I can always covert the string into int64_t, but I thought I would ask if this is intended.

This comment has been minimized.

@grondo

grondo Mar 17, 2018

Author Contributor

Sorry, I thought that check_jobid_arg() converted id to a Lua number and thus the value would be encoded as int64. I'll fix this up for you and sorry I didn't catch it!

This comment has been minimized.

@dongahn

dongahn Mar 17, 2018

Contributor

Not a problem! This is just who will do the conversion.

This comment has been minimized.

@grondo

grondo Mar 17, 2018

Author Contributor

For now you can try changing the code to

  f:rpc ("sched.cancel", { jobid = tonumber (id) }

And see if that works

Otherwise, I'll be back online later to fix this for you.

@grondo grondo deleted the grondo:wreck-cancel branch Mar 19, 2018

@grondo grondo referenced this pull request May 10, 2018

Closed

0.9.0 Release #1479

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.