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

misc cleanups & minor fixes #2360

Merged
merged 9 commits into from Sep 11, 2019

Conversation

@chu11
Copy link
Contributor

commented Sep 10, 2019

Various misc cleanups & minor fixes peeled off of my now closed PRs (#2359, #2353, #2352).

Only one of note is the refactoring I did which added "flags" support to job-info event watches. Even though the flags are not used, I figure we can keep it just in case for the future.

chu11 added 9 commits Sep 6, 2019
Use same variable names and patterns for calling some internal
functions.
Output file name was not incremented correctly compared to prior
output file names.
Add comments on corner case behavior when a guest namespace
is destroyed.
shell_task_io_enable() was removed in

5492d5c

remove lingering references to it in header and comments.
Add corner case check to ensure guest output header is read before
any data.
Add flags for job-info.eventlog-watch and
job-info.guest-eventlog-watch rpc calls for potential future
feature support.

In addition, support flags argument in flux_job_event_watch()
in libjob.
On completion of watching the output eventlog, cancel the eventlog watch instead
of simply destroying the future and exiting.  This should avoid a potential
matchtag leak.
During guest watch state transitions, there is a small racy scenario
in which a user may cancel an event watch while the event watch is
in the process or returning an error (ENOTSUP, etc.).  Detect
this cancellation and respond to the cancellation instead of moving
onto the next state.
@chu11 chu11 force-pushed the chu11:misc_cleanup14 branch from f0f8429 to 94c9a78 Sep 11, 2019
@codecov-io

This comment has been minimized.

Copy link

commented Sep 11, 2019

Codecov Report

Merging #2360 into master will decrease coverage by 0.03%.
The diff coverage is 74.07%.

@@            Coverage Diff            @@
##           master   #2360      +/-   ##
=========================================
- Coverage   80.84%   80.8%   -0.04%     
=========================================
  Files         218     218              
  Lines       34501   34514      +13     
=========================================
- Hits        27891   27889       -2     
- Misses       6610    6625      +15
Impacted Files Coverage Δ
src/shell/task.c 79.16% <ø> (ø) ⬆️
src/shell/output.c 76.62% <ø> (ø) ⬆️
src/modules/job-info/watch.c 69.04% <100%> (+0.18%) ⬆️
src/common/libjob/job.c 77.19% <100%> (ø) ⬆️
src/modules/job-info/guest_watch.c 71.59% <64.28%> (-0.01%) ⬇️
src/cmd/flux-job.c 85.12% <77.77%> (-0.19%) ⬇️
src/common/libflux/mrpc.c 87.79% <0%> (-1.19%) ⬇️
src/modules/connector-local/local.c 73.26% <0%> (-1.16%) ⬇️
Copy link
Member

left a comment

LGTM! Please set merge-when-passing when you're ready.

@mergify mergify bot merged commit c02631a into flux-framework:master Sep 11, 2019
3 of 4 checks passed
3 of 4 checks passed
codecov/patch 74.07% of diff hit (target 80.84%)
Details
Summary 1 rule matches
Details
codecov/project 80.8% (-0.04%) compared to 9391b20
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@garlick garlick referenced this pull request Sep 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.