Skip to content

Update to pg 2.17#2

Merged
pstaabp merged 4 commits into
openwebwork:developfrom
drgrice1:update-to-pg-2.17
Jul 23, 2022
Merged

Update to pg 2.17#2
pstaabp merged 4 commits into
openwebwork:developfrom
drgrice1:update-to-pg-2.17

Conversation

@drgrice1

Copy link
Copy Markdown
Member

Update the standalone renderer to use pg version 2.17.

The entire lib/WeBWorK/htdocs directory has been removed. The few files that are needed by the standalone renderer are now in public/Problem and public/PGCodeMirror.

To make this work, static file handling had to be revises. See lib/RenderApp/Controller/StaticFiles.pm. Basically, the only files that are served from webwork2_files are CAPA_graphics files (now served directly from Contrib/CAPA/CAPA_graphics) or tmp files (served from a directory in the app root name tmp). PG files are served from the pg_files location (files are in lib/PG/htdocs).

A getAssetURL method is in lib/WeBWorK/lib/WeBWorK/Utils.pm much like webwork2's method handles the correct redirection of the pg_files.

Note that codemirror is no longer serving their javascript and css files. So temporarily this has add an npm install step in the root directory of the renderer. A vendor link in public makes those files available. We can either switch to another CDN to serve those files or go with this. I much prefer this. CDN's are not reliable. I think it would be best to switch all third party javascript to using this approach.

Note that MathQuill is now optional much the same way as in #83, but the default is that it is enabled. Of course, the standalone renderer no longer adds MathQuill itself as PG now does that.

I recommend running git diff develop :^lib/WeBWorK/htdocs from this branch to see the changes without the lengthy list of deletions from the lib/WeBWorK/htdocs directory.

there that are still needed by the standalone renderer when rendering a
problem.  Requests to webwork2_files are redirected as appropriate by
the renderer.  Most go to PG/htdocs.
@drgrice1 drgrice1 requested a review from drdrew42 as a code owner July 19, 2022 03:08
@drgrice1

Copy link
Copy Markdown
Member Author

This repository really needs to become the primary repository for the standalone renderer, and not @drdrew42's repository. The same needs to be true of the OPLv3 repository if these are going to be considered for use with webwork.

@drdrew42 drdrew42 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested and everything works great!
Caveats:

  • make sure Mojolicious is updated (and Future::AsyncAwait as well), needs 9.17+
  • git submodule update (for PG)
  • npm install x2 (for app AND for PG)

@taniwallach

Copy link
Copy Markdown
Member

This repository really needs to become the primary repository for the standalone renderer, and not @drdrew42's repository. The same needs to be true of the OPLv3 repository if these are going to be considered for use with webwork.

Could the ownership of the root level repository be transferred to openwebwork?

@taniwallach taniwallach left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I tested this at and posted comments about the Docker build issues I ran into at drdrew42#89 (comment) and some general comment at drdrew42#89 (comment).

This works and even supports right-to-left (Hebrew) problems better than was the case in the old version where a special rtl.css was added (via a local hack).

@drgrice1

Copy link
Copy Markdown
Member Author

@taniwallach: What do you mean by root level ownership? Do you mean the github reference that this is forked from drdrew42/renderer?

@taniwallach

Copy link
Copy Markdown
Member

@taniwallach: What do you mean by root level ownership? Do you mean the github reference that this is forked from drdrew42/renderer?

Yes, and more. The tree shown at https://github.com/openwebwork/renderer/network/members would also then show openwebwork as the "root" of the tree of forks.

I am more interested is trying to have the PR history, discussions, etc. move over to this repository if that is possible. AFAIK a transfer of repository ownership in GitHub would keep all that as part of what would be available.

This was referenced Jul 22, 2022
@pstaabp

pstaabp commented Jul 22, 2022

Copy link
Copy Markdown
Member

Testing the local version of this and started fresh (deleting the old directory). A few things:

  • Since the openwebwork will be the primary repository, the README instructions should reflect this. I think the only change is to switch drdrew42 to openwebwork on line ### of README.md.
  • testing on webwork3, I have a couple of issues:
    • The proxy on devServer (quasar.conf.js) needed to be changed to
      '/renderer': {
          target: 'http://localhost:3001',
          pathRewrite: {
         '^/renderer': ''
         }
      },```
      clearly, this is easy to fix on ww3, but I did the routes change? or did I not set up the conf file correctly?
      
    • I'm getting 404 errors when getting mathquill.js (from the pg side) and problem.scss. This might be related to the conf file. If so, maybe add some additional instructions in the README.

@drgrice1

Copy link
Copy Markdown
Member Author

@pstaabp: There is no need to change the quasar.conf.js for webwork3. Check your render_app.conf file.

@drdrew42

Copy link
Copy Markdown
Member

@pstaabp asset URLs should now be fixed -- I had a similar issue with requests being misrouted. No need to change anything else, just pull the latest updates to this branch.

@drdrew42

drdrew42 commented Jul 22, 2022

Copy link
Copy Markdown
Member

Also, be sure to run npm install in both the renderer root and lib/PG/htdocs

@pstaabp

pstaabp commented Jul 22, 2022

Copy link
Copy Markdown
Member

With the newest changes, everything is working out of the box, except the following warning:

Use of uninitialized value in substitution iterator at /opt/webwork/renderer/lib/RenderApp/Controller/FormatRenderedProblem.pm line 307.

@drgrice1

Copy link
Copy Markdown
Member Author

@pstaabp: The use of uninitialized value warnings are caused by the variables $LTIGradeMessage and $problemSeed being in the ww3_format.pl file, but never defined in the preceding code of FormatRenderedProblem.pm. The same things happens for the other formats when using the standalone renderer directly. Unfortunately the substitution s/(\$\w+)/"defined $1 ? $1 : ''"/gee does not work to avoid those warnings. I think it may have been believed that by using the defined $1 ? $1 : '' part it would avoid that but it doesn't. In any case, this is not caused by this pull request.

@pstaabp pstaabp merged commit 47a0952 into openwebwork:develop Jul 23, 2022
@drgrice1 drgrice1 deleted the update-to-pg-2.17 branch August 2, 2022 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants