t1011-job-archive-interface.t relies on deprecated module

[garlick]

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.

Possibly it's too early to remove this module?

If we can remove it, but we need to keep tests for migration around for a while we could maybe do something like this

index a2f18ea..726b5f1 100755
--- a/t/t1011-job-archive-interface.t
+++ b/t/t1011-job-archive-interface.t
@@ -4,6 +4,20 @@ test_description='test fetching jobs and updating the fair share values for a gr

 . $(dirname $0)/sharness.sh

+have_module() (
+    eval $(flux env)
+    IFS=:
+    for dir in "$FLUX_MODULE_PATH"; do
+        test -f $dir/$1 && return 0
+    done
+    return 1
+)
+
+if ! have_module job-archive.so; then
+    skip_all="Skipping job-archive tests as module is unavailable"
+    test_done
+fi
+
 DB_PATH=$(pwd)/FluxAccountingTest.db
 ARCHIVEDIR=`pwd`
 ARCHIVEDB="${ARCHIVEDIR}/jobarchive.db"

I wasn't sure if the test was covering other important things as well.

Thoughts?

[cmoussa1]

I think the main reason for keeping the job-archive load in the test suite (specifically in this file) was just for testing that the flux-accounting script that was responsible for fetching inactive jobs had the capability of querying the job-archive DB if needed. :-) I think it would probably be okay to remove the module if you guys wanted to. I could edit t1011-job-archive-interface.t to no longer test querying the job-archive DB for job records. What do you think?

[garlick]

Accounting is the only user, so if it's past needing it then it probably makes sense to remove the module in core.

If the test script is doing other things, the diff above could be modified to do a

if have_module job-archive.so; then
    test_set_prereq JOB_ARCHIVE
fi

and then any sub-tests that require the module could declare that they need that prereq e.g.

test_expect_success JOB_ARCHIVE 'test description...'

t/python/t1006_job_archive.py would need attention too.

[garlick]

No urgency here btw.

[cmoussa1]

OK cool! I think I can pretty easily just edit the tests to not populate the job-archive module's DB. That way we don't have to add a skip to a couple of the tests? Unless we want to keep those tests in the instance where we are rolling out or testing flux-accounting on a system with an older version of flux-core? I'm pretty indifferent on either approach and would defer to your expertise.

t/python/t1006_job_archive.py would need attention too.

Ah, I actually think that test might be okay - that no longer tests querying the job-archive DB; I think I should edit a few of the comments and test description to make it more clear, though.

[garlick]

Unless we want to keep those tests in the instance where we are rolling out or testing flux-accounting on a system with an older version of flux-core?

Just spot checked corona and it's running flux-accounting-0.37.0 (TOSS 4.8.4). I think the local job db went in in release 0.33.0? (judging from release notes). I don't know if we can conclude from that that all the production systems are well past 0.33.0 and are no longer using job-archive? Perhaps @ryanday36 can comment.

That python test is failing with this in the log when I run it against the flux-core branch that removes job-archive. Not sure why?

Traceback (most recent call last):
  File "/home/garlick/proj/flux-accounting/t/./python/t1006_job_archive.py", line 20, in <module>
    from fluxacct.accounting import job_usage_calculation as jobs
  File "/home/garlick/proj/flux-accounting/src/bindings/python/fluxacct/accounting/job_usage_calculation.py", line 15, in <module>
    from fluxacct.accounting import jobs_table_subcommands as j
  File "/home/garlick/proj/flux-accounting/src/bindings/python/fluxacct/accounting/jobs_table_subcommands.py", line 16, in <module>
    from flux.resource import ResourceSet
ModuleNotFoundError: No module named 'flux'
FAIL python/t1006_job_archive.py (exit status: 1)

[cmoussa1]

Hmmm. Here is how that file is trying to import the flux module:

from flux.resource import ResourceSet

I'm not sure if this means it is assuming a global install of flux and its Python bindings? The Python bindings for the accounting module have this defined in its Makefile.am:

install-data-hook:
    $(AM_V_at)echo Linking python modules in non-standard location... && \
      $(INSTALL) -d -m 0755 "$(DESTDIR)$(fluxpylinkdir)" && \
      target=$(fluxpydir) && \
      f=$${target##*/} && \
      cd "$(DESTDIR)$(fluxpylinkdir)" && \
      rm -f $$f && \
      $(LN_S) $$target .

Sorry if this is not helpful.

[garlick]

Oh maybe it's because I had built a deb for that flux-core branch and installed it. Unrelated problem then!

[cmoussa1]

After some digging and quick testing in my Docker environment with adding the skips for specific tests (where some fail because they assume the skipped tests will succeed), I wonder if the following would be a good approach:

• If the job-archive module does not exist, I should skip the entire t1011-job-archive-interface.t
• I should add another test file that does not rely on testing fetching records from the job-archive module at all and run through similar exercises as in t1011-job-archive-interface.t, since there is only one other place in the test suite where flux account-fetch-job-records.py is called

What do you think?

[garlick]

Sounds good to me!

[cmoussa1]

This should be fixed by #518 now! Closing.