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

Add flux_job_kvs_namespace(3) and use it in job-exec for guest namespace #2315

Merged
merged 7 commits into from Aug 16, 2019

Conversation

@grondo
Copy link
Contributor

commented Aug 16, 2019

As discussed #2313, this PR adds a new flux_job_kvs_namespace(3) function to libjob, a flux-job namespace command, and uses flux_job_kvs_namespace for naming the guest namespace for jobs. The guest namespace is now job-<id>.

Fixes #2306

grondo added 6 commits Aug 15, 2019
Collect "buffer args" checks into a single function to avoid code
duplication.
Add a function to write the standard guest namespace for jobs into
a buffer, to avoid users hand-coding an expected and possibly changing
guest namespace name in scripts/code/etc.
Add simple tests for flux_job_kvs_namespace()
Improve error usefulness by including the argument that failed
to parse as unsigned in flux-job's parse_arg_unsigned() helper
function.
Add a subcommand that converts flux job ids to namespace names,
giving users and scripts a standard way to get kvs guest namespace
names for jobs.
Add some sanity checks for the flux-job namespace subcommand.
@grondo grondo force-pushed the grondo:job-kvs-namespace branch from 2332ea0 to 0980129 Aug 16, 2019
Copy link
Contributor

left a comment

overall looks good to me, just noticed the one typo in the commit message

@@ -843,13 +843,6 @@ static flux_future_t *jobinfo_start_init (struct jobinfo *job)
return NULL;

This comment has been minimized.

Copy link
@chu11

chu11 Aug 16, 2019

Contributor

typo in commit message "notiation" -> "notation"

This comment has been minimized.

Copy link
@grondo

grondo Aug 16, 2019

Author Contributor

Thanks! fixed the typo an force-pushed.

Switch to using libjob's flux_job_kvs_namespace() to create job
guest kvs namespace names, instead of using the dothex notation
of a flux_jobid_t.

This puts job namespace name generation in a single place so that
these names do not have to be generated manually anywhere, as
well as allowing the naming convention to be updated in the future
without breaking any existing code.

As a side effect, this fixes a problematic side effect of kvs
namespace names with periods in them, since these names are
used as keys in kvs module stats output.

Fixes #2306
@grondo grondo force-pushed the grondo:job-kvs-namespace branch from 0980129 to f30da5a Aug 16, 2019
@codecov-io

This comment has been minimized.

Copy link

commented Aug 16, 2019

Codecov Report

Merging #2315 into master will increase coverage by 0.03%.
The diff coverage is 95.83%.

@@            Coverage Diff             @@
##           master    #2315      +/-   ##
==========================================
+ Coverage   80.87%   80.91%   +0.03%     
==========================================
  Files         214      214              
  Lines       33800    33815      +15     
==========================================
+ Hits        27337    27362      +25     
+ Misses       6463     6453      -10
Impacted Files Coverage Δ
src/modules/job-exec/job-exec.c 74.67% <100%> (-0.06%) ⬇️
src/common/libjob/job.c 76.87% <100%> (+1.71%) ⬆️
src/cmd/flux-job.c 85.4% <92.85%> (+0.14%) ⬆️
src/common/libflux/mrpc.c 87.79% <0%> (-1.19%) ⬇️
src/common/libflux/message.c 80.88% <0%> (+0.12%) ⬆️
src/cmd/flux-module.c 83.96% <0%> (+0.23%) ⬆️
src/modules/connector-local/local.c 74.71% <0%> (+1.44%) ⬆️
@garlick

This comment has been minimized.

Copy link
Member

commented Aug 16, 2019

LGTM!

@garlick garlick merged commit a70be75 into flux-framework:master Aug 16, 2019
4 checks passed
4 checks passed
Summary 1 potential rule
Details
codecov/patch 95.83% of diff hit (target 80.87%)
Details
codecov/project 80.91% (+0.03%) compared to 0b39b27
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@grondo grondo deleted the grondo:job-kvs-namespace branch Aug 16, 2019
@grondo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 16, 2019

Thanks!

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