Reduce mercurial work in the tool shed's tool source and install info paths#23181
Draft
mvdbeek wants to merge 2 commits into
Draft
Reduce mercurial work in the tool shed's tool source and install info paths#23181mvdbeek wants to merge 2 commits into
mvdbeek wants to merge 2 commits into
Conversation
_shed_tool_source_for cloned the tool shed's own repository over HTTP, through the shed's own hgweb front end, into a temporary directory just to read a single tool XML - while already holding the local repository handle. Replace the clone with a local 'hg archive' of the target revision, which needs no network round trip and no changelog history. The whole revision is still materialized rather than just the tool file, because tool XML may import macros from sibling files. repository_clone_url is no longer needed to materialize a revision, so drop it from the manager functions that only threaded it through.
get_changectx_for_changeset walked the entire changelog, materializing a changectx and hex-formatting it for every revision, to find one hash. Resolve the hex node id prefix directly instead. Note repo[changeset_revision] cannot be used for this - it resolves revision symbols, so an all-digit hash such as "123456789012" would be read as a revision number. The null node and short prefixes are rejected to match what the changelog scan matched. get_repo_info_dict, on the get_install_info path, then no longer needs to fork 'hg id' via changeset2rev to determine ctx_rev. get_next_downloadable_changeset_revision fetched the downloadable revisions from the database and then walked the changelog again to recover ordering it had already asked for. Order the downloadable revisions by their changelog position directly. RepositoryMetadata.numeric_revision is deliberately not used for this: it is not updated when a changeset_revision is moved ahead to a new repository tip (repository_metadata_manager.py), so it can disagree with the real changelog position.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reduces the mercurial work the tool shed does when serving tool sources and install info. Two independent changes.
Materialize tool revisions locally instead of cloning over HTTP
_shed_tool_source_forcloned the tool shed's own repository over HTTP, through the shed's own hgweb front end, into a temporary directory — just to read a single tool XML. It did this while already holding the local repository handle and the changectx two lines above:That's a network round trip plus a full changelog transfer per call. It now does a local
hg archiveof the target revision viaarchive_repository_revision, which was already intool_shed/util/hg_util.pybut had no callers.The whole revision is still materialized rather than just the tool file, because tool XML may import macros from sibling files — a single-file extraction would silently break every macro-based wrapper. There's a new
column_maker_with_macrosfixture and test covering that.repository_clone_urlwas only ever needed to feed the clone, so it's dropped from the manager functions that threaded it through.Worth noting for reviewers:
parsed_tool_model_cached_forcaches the parsed model, butGET /api/tools/{tool_id}/versions/{tool_version}/tool_sourcecallstool_source_fordirectly and bypasses that cache, so it paid the clone on every request.Resolve changeset revisions without scanning the changelog
get_changectx_for_changesetwalked the entire changelog, materializing a changectx and hex-formatting it for every revision, to find one hash:It now resolves the hex node id prefix directly. Note that
repo[changeset_revision]cannot be used here — it resolves revision symbols, so an all-digit hash such as123456789012would be read as a revision number. The null node and short prefixes are rejected so the result matches what the changelog scan matched. There are 8 call sites.Two consequences on the
get_install_infopath:get_repo_info_dictno longer forkshg idviachangeset2revto computectx_rev.get_next_downloadable_changeset_revisionfetched the downloadable revisions from the database and then walked the changelog again to recover ordering it had already asked for. It now orders them by changelog position directly. This one is called once per transitive dependency duringRelationBuildertraversal.A trap worth flagging
RepositoryMetadata.numeric_revisionlooks like the obvious source forctx_rev— it's indexed, already populated, and means exactly that. It is not safe to use.repository_metadata_manager.py:1140moveschangeset_revisionahead to a new repository tip without updatingnumeric_revision, so the two can disagree. Thecolumn_maker_with_download_gapsfixture has a row withnumeric_revision=1whose real changelog position is2. Since Galaxy feedsctx_revstraight intohg clone -r, trusting that column would install the wrong revision.This also means
get_metadata_revisionssorts by a column that can be stale, which propagates toget_latest_downloadable_changeset_revision. Not addressed here — fixing it properly needs a data backfill, not just a write at the move-ahead site. Happy to open a separate issue.How to test the changes?
(Select all options that apply)
New
test/unit/tool_shed/test_hg_revision_lookup.pycovers changeset resolution across both fixtures (including null-hash, short-prefix and unknown-hash rejection), next-downloadable ordering over the gap fixture, and thatget_repo_info_dict'sctx_revequals the real mercurial revision — that last one is the regression guard for thenumeric_revisiontrap above.test/unit/tool_shed/test_tool_source.pygains a macro-expansion case.License