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

job-archive: remove module #6378

Merged
merged 1 commit into from
Oct 28, 2024
Merged

Conversation

garlick
Copy link
Member

@garlick garlick commented Oct 16, 2024

@garlick
Copy link
Member Author

garlick commented Oct 17, 2024

oops - looks like there are still some flux-accounting tests that rely on this.

2024-10-17T00:00:13.5772402Z FAIL: t1011-job-archive-interface.t 7 - load job-archive module

@chu11
Copy link
Member

chu11 commented Oct 17, 2024

How about adding to the commit message that this fixes ... #5288

cmoussa1 added a commit to cmoussa1/flux-accounting that referenced this pull request Oct 17, 2024
Problem: t1011-job-archive-interface.t relies on the job-archive module
in flux-core which is being considered for removal in
flux-framework/flux-core#6378 since flux-accounting now has that
capability on its own.

Add a check in t1011-job-archive-interface.t that will skip the tests in
the event that the job-archive module does not exist.
cmoussa1 added a commit to cmoussa1/flux-accounting that referenced this pull request Oct 17, 2024
Problem: t1011-job-archive-interface.t relies on the job-archive module
in flux-core which is being considered for removal in
flux-framework/flux-core#6378 since flux-accounting now has that
capability on its own.

Add a check in t1011-job-archive-interface.t that will skip the tests in
the event that the job-archive module does not exist.
Copy link

codecov bot commented Oct 17, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.55%. Comparing base (0ed5463) to head (c50d761).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6378      +/-   ##
==========================================
+ Coverage   83.29%   83.55%   +0.26%     
==========================================
  Files         524      523       -1     
  Lines       86230    87348    +1118     
==========================================
+ Hits        71826    72985    +1159     
+ Misses      14404    14363      -41     

see 302 files with indirect coverage changes

cmoussa1 added a commit to cmoussa1/flux-accounting that referenced this pull request Oct 17, 2024
Problem: t1011-job-archive-interface.t relies on the job-archive module
in flux-core which is being considered for removal in
flux-framework/flux-core#6378 since flux-accounting now has that
capability on its own.

Edit the tests that use the job-archive module and just use
flux-accounting's fetch-job-records script to fetch inactive job
records.
Copy link
Contributor

@grondo grondo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@garlick garlick added this to the flux-core-0.68.0 milestone Oct 28, 2024
Problem: the job archive module is no longer needed now that
flux-accounting maintains its own historical job record internally.

Remove job-archive, its tests, and its documentation.

Fixes flux-framework#5288
@garlick
Copy link
Member Author

garlick commented Oct 28, 2024

Just force pushed after rebasing on current master and caching a couple of dangling man page references in the docs. Will set MWP.

@mergify mergify bot merged commit 13fb49a into flux-framework:master Oct 28, 2024
31 checks passed
@garlick garlick deleted the kill_archive branch October 28, 2024 23:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants