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

modules/cron: misc cleanup #1657

Merged
merged 6 commits into from Sep 12, 2018

Conversation

Projects
None yet
3 participants
@chu11
Copy link
Contributor

commented Sep 12, 2018

In #1564, modules/cron was converted to use the new subprocess library. While everything works, some code seems wrong/out of place/wrong style given differences between the old and new subprocess library. This PR cleans up / adjusts modules/cron accordingly.

@coveralls

This comment has been minimized.

Copy link

commented Sep 12, 2018

Coverage Status

Coverage increased (+0.03%) to 79.413% when pulling 5baaad6 on chu11:issue1331-cleanup into 6934d50 on flux-framework:master.

@chu11

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2018

hit a #1641, restarting builder

chu11 added some commits Aug 30, 2018

modules/cron: Add task completed flag
Indicates that subprocess has "completed", i.e. exited & I/O complete
modules/cron: Add task exec_failed state flag
This flag indicates state "Exec Failed" was reached, and differentiates
it from a general process exiting.
modules/cron: Remove task stdout/err closed flags
With the new flux subprocess API and completed flag, the stdout/err
closed flags are no longer needed.
modules/cron: Rename "completed" functions/vars
Rename all functions and variables that use the word "completed"
or "completion" in it.  Within the new flux subprocess library
"completed" means that the subprocess has exited and I/O is complete.
However, within the cron task library, "completed" means "no longer
running", which includes no longer running due to error situations.
Instead use the name "finished" for this situation.

Following have been renamed.

cron_task_completed -> cron_task_finished
cron_task_handle_completion -> cron_task_handle_finished
cron_task_completed_f -> cron_task_finished_f
completion_cb -> finished_cb
cron_entry_completion_handler -> cron_entry_finished_handler
completed_tasks -> finished_tasks
cron_entry_push_completed_task -> cron_entry_push_finished_task
modules/cron: Remove 'eof' flag in io_cb
The eof flag no longer makes sense as zio is no longer used.
modules/cron: cache cwd
On module load, cache current working directory, to avoid constant
lookups when tasks are launched.

@chu11 chu11 force-pushed the chu11:issue1331-cleanup branch from 317bdf8 to 5baaad6 Sep 12, 2018

@chu11

This comment has been minimized.

Copy link
Contributor Author

commented Sep 12, 2018

rebase and repush

@grondo

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2018

Nice, since this naturally follows #1564, merging now.

@grondo grondo merged commit b7b5e18 into flux-framework:master Sep 12, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.03%) to 79.413%
Details
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.