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

add optional external runestone methods for the CLI to hook into #2348

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

oscarlevin
Copy link
Member

This slightly refactors the runestone services logic, creating a new function that does the downloading of the services xml file, and adding hooks to the html and dynamic_substitutions functions. PreTeXt expects this function to either be None or to have keyword arguments, including a format that can be either "xml" or "tgz" to distinguish whether it should just do the services query (xml) or get the entire tgz file.

Behavior of the pretext script should be unchanged when ext_rs_methods is None.

Happy to rename variables if you have a better suggestion.

@oscarlevin
Copy link
Member Author

Just force-pushed a fix so that dynamic_substitutions uses the same hook, and for the pretext script to use the correct signatures.

@rbeezer
Copy link
Collaborator

rbeezer commented Jan 3, 2025

Just force-pushed

You read my mind. I meant to comment yesterday, but it got away from me.

OK, no more time today to investigate, but in the interest of moving this along. I think Brad uses some debug.* Runestone features for testing, perhaps with the CLI. Will the setup here still allow that, without you duplicating the debug logic in the external_* function?

@oscarlevin
Copy link
Member Author

There are two places that the external functions are used.

  1. Instead of downloading the .tgz file (the external function could provide a local copy if already there, or download and cache the requested copy if possible). This is inside _place_runestone_services (line 3599)
  2. Instead of querying the runestone services URL for the xml file with basic info. This is in the _runestone_services function, line 3489. (And in reading that, I noticed a stray debugging print statement I just removed and force-pushed.)

So I don't think there is any way that a debug switch could be affected by this. In particular, in the presence of a network connection and Runestone being up, the result should be identical to any build.

@oscarlevin
Copy link
Member Author

This now includes the fix for debug.rs.version, whose presence will prevent the external method from being called.

@oscarlevin
Copy link
Member Author

Just added a commit that changes the timeout for the requests. Instead of 10 seconds for both connecting and reading the contents, this now timeouts after 1 second for the initial connection, but still allows 10 seconds to complete the task. That way, if there is no response, we know sooner.

@oscarlevin
Copy link
Member Author

Heads up; I'm going to refactor this PR to make it easier for the external method to give back the correct info.

@oscarlevin
Copy link
Member Author

All done. Now a single commit with the complete changes described in the comments above.

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.

2 participants