creation of a script to update related to the OPL

[pstaabp]

This script allows to rerun a few scripts related to the OPL but doesn't require the entire OPLupdate script to be run. These scripts produce JSON files that

1. have the subject-chapter-section information
2. textbook information
3. directory information

and are mainly used by the ww3 branch.

To test, the OPL update script should have been run and then the following

updateOPLextras --textbooks
updateOPLextras --directories
updateOPLextras --subjects
updateOPLextras --all

and the results should say that a file in the HTDOCS directory was successfully produced.

[jwj61]

I haven't tested it and probably won't have a chance to (I am about to go on vacation), but the code looks ok.

[taniwallach]

Issues:

1. When I installed file files, bin/updateOPLextras was not executable.
2. Please git mv it to be bin/updateOPLextras.pl.
3. It seems to depend on MooX/Options.pm:

• For Docker we should add libmoox-options-perl to the list of Ubuntu packages installed in Dockerfile.
• It should also be added to bin/check_modules.pl
• The changes for item 3 are handled by Need MooX::Options for webwork2 PR #959 #989

After installing libmoox-options-perl by hand in a running container (or using the changes from #989) 2 of the individual commands worked (without much of a wait):

updateOPLextras --textbooks
updateOPLextras --directories

and the last one was taking quite some time to run,and in the end I stopped it and did not give it a chance to complete. That will be needed in the next round of testing.

updateOPLextras --subjects

Since the updateOPLextras --all just runs all 3 subroutines sequentially, it should also be fine, and I'm not going to test it (as the subjects run is very CPU/time intensive).

Note: The code also changes to contents of the 3 JSON files:

• textbook-tree.json
• library-directory-tree.json
• library-subject-tree.json (This one takes lots of time to be generated.)
  but as @pstaabp noted, these files are only used by the WW3 codebase, and not by the regular codebase - so those changes should not effect anyone, except users of the WW3 codebase, who will need to use the correct format of those files for their version of WW3.

[taniwallach]

When I tried to add MooX::Options to bin/check_modules an error is reported unless I also added Moo before it in the module list to be tested:

** MooX::Options found, but failed to load: Can't find the method in

! Ensure to load a Role::Tiny compatible module like Moo or Moose before using MooX::Options. at (eval 452) line 1.
BEGIN failed--compilation aborted at (eval 452) line 1.

Solution is to include Moo before MooX::Options.

[taniwallach]

#989 adds the changes needed to install and test for install of Moox::Options.

[mgage]

I've tried to be conservative about adding lots of packages. Adding Moose in particular would add too much bloat to the webwork system. Moo might be ok. I've already added Class::Accessor and used for constructing the answerTable for each problem which might be enough for the usage here. That's already in our code base so nothing new would need to be added.

There is a useful comparison of perl OO options at the end of
https://mojolicious.org/perldoc/perlootut.

From that I'd recommend rewriting this using either Class::Accessor or Class::Tiny. There might need to be a little extra code to deal with the options feature -- I haven't looked closely at that.

[mgage]

I closed this prematurely by mistake.

[Alex-Jordan]

This points into the WW-2.15 branch, not develop. @pstaab, would you assess this for closing? Or should it move to develop?

[drgrice1]

Make sure to make the updateOPLextras file executable, and rename it to updateOPLextras.pl (for consistency with the other OPL scripts) as @taniwallach has said.

[pstaabp]

I'm embarrassed that I left this sit so long. I made changes suggested by @drgrice1 and switched it to be a .pl instead of executable.

Digging into this again, it was running super slow and have trimmed down the database queries to speed this this, so there's a lot of code changes, mainly to streamline. I'll commit this later today after more testing.

Another important thing: the updateOPLextras script calls functions in OPLUtils.pm and OPL-update also calls these. My thought is to pull these calls out of OPL-update. I think the JSON files that are produce were only used in webwork3 (so I'm not sure how it ended up in OPL-update and if these need to be generated, then the updateOPLextras script can be run.

Thoughts?

[drgrice1]

You say that you added the .pl extension, but did not make the script executable? Both are needed. If anything, making the script executable is more important than adding the .pl extension. Adding the extension is purely for show, and to maintain consistency in naming with the other scripts. Making the script executable actually makes it so that you can just type the filename on the command line to execute it without adding perl before the filename.

[pstaabp]

I haven't pushed anything yet. I'm still bug testing.

I am interested in seeing people's opinion about pulling the functionality out of OPL-update though.

[pstaabp]

I just pushed an update and could use more testing. A few things about this

• updateOPLextras.pl is now executable.
• There are both long and short versions of all switches. updateOPLupdate.pl --help gives all info on this.
• if testing, make sure you use the --verbose flag, just added this. Without it, you won't see output.
• The --textbooks switch takes a few minutes to run. I'll see if I can improve this as well.

Also, I haven't updated the OPL-update script, but I think it's a good idea to separate the building of the OPL from this functionality.

[drdrew42]

From that I'd recommend rewriting this using either Class::Accessor or Class::Tiny. There might need to be a little extra code to deal with the options feature -- I haven't looked closely at that.

It seems that the only purpose of including Moo is to use MooX::Options to handle the parsing of command line arguments. For this we already have Getopt::Long, so it's not necessary to add these new libraries.

use strict;
use warnings;
use DBI;
use Getopt::Long;
Getopt::Long::Configure ("bundling");

my ($textbooks, $directories, $subjects, $all);
GetOptions (
  't|textbooks'   => \$textbooks,
  'd|directories' => \$directories,
  's|subjects'    => \$subjects,
  'a|all'         => \$all,
);

Getopt::Long has abbreviation of parameters by default, but specifying the single-character options allows for bundling - so that combinations of two of the three available options can be selected in one go.

If you want the additional documentation that is included in MooX::Options, Pod::Usage is well-coupled with Getopt::Long:

=head1 NAME
 
updateOPLextras - re-build library trees
 
=head1 SYNOPSIS
 
updateOPLextras [options]
 
 Options:
   -t --textbooks        (rebuild textbook tree)
   -s --subjects         (rebuild subject tree)
   -d --directories      (rebuild directory tree)
   -a --all              (rebuild all trees)
   -h --help             (display this text)
 
=head1 OPTIONS
 
=over 8
 
=item B<-t> I<--textbooks>
 
Rebuild the textbook tree and write to a JSON file.

=item B<-s> I<--subjects>

Rebuild the subject tree and write to a JSON file.

=item B<-d> I<--directories>

Rebuild the directory tree and write to a JSON file.
  
=back
 
=head1 DESCRIPTION
 
B<This program> will rebuild the specified library trees
from the existing library contents in the database.
 
=cut

use strict;
use warnings;
use DBI;
use Getopt::Long;
use Pod::Usage;
Getopt::Long::Configure ("bundling");

my ($textbooks, $directories, $subjects, $all);
GetOptions (
  't|textbooks'   => \$textbooks,
  'd|directories' => \$directories,
  's|subjects'    => \$subjects,
  'a|all'         => \$all,
);
pod2usage(2) unless ($textbooks || $directories || $subjects || $all);

[drgrice1]

I agree with @drdrew42 to use Getopt::Long instead of pulling in an unnecessary new module.

I think it is a good idea to pull the things out of OPL-update that are not used for webwork2. They can be added back as needed when we migrate to webwork3. The OPL-update script takes long enough to run as it is, and that may cut down the run time a bit.

While you are at it you should change the to_json calls either to encode_json or to using the object oriented approach. I think that just using encode_json is the correct thing to do. The pretty option that is currently being used should be dropped. That is inefficient and that should not be used here. We really should not be going for human readable json.

[pstaabp]

I'll switch over to Getopt::Long. I've been using MooX because it's a backbone of Dancer2, so I don't think of it as a new package, but in webwork2 I guess it is.

[pstaabp]

Updated this script to include recommendations. Note: I also pulled out the functionality from OPL-update. I'm in the middle of testing this, but I don't see anything I removed that will change the base OPL-update functionality. This should speed up the script.

[drgrice1]

There is an instance of to_json on line 537 of OPL-update. It would be good to take care of that now while you are at it. You could export writeJSONtoFile from OPLUtils.pm and import and use that.

Also, could you remove libmoox-options-perl from Dockerfile (line 200) and Moo and MooX::Options from lines 90-91 of check_modules.pl now that they are not used?

I question the need of this pull request. If the json files generated by updateOPLextras.pl are not used by webwork2 at all, then why is this here? If this is only for webwork3, then this does not belong. There is a lot of code that is in webwork2 that seems to be for webwork3, and this should not be occuring. webwork3 should have its own repository entirely, and should not be mingled with webwork2. This needs to be rethought.

[drgrice1]

Lines 78 and 80 above should be deleted. Those variables are not needed here.

[drgrice1]

Let me clarify that I think it is good that this pull request is now removing those things needed for webwork3 from the script that is run for webwork2.

[pstaabp]

I'm sure the extra utilities in OPL-update script was my fault, although I remember suggesting that the Library Browser in webwork2 use the JSON files generated to improve performance. That never occurred.

@drgrice1 Made the changes.

[drgrice1]

This looks good now.

[drgrice1]

It would be good for someone else to take a look at this before it is merged.