Repository navigation
Conversation
jat255
added
r
Contributor
|
Preview root: https://posit-dev.github.io/commons/pr-436/ Python site preview: https://posit-dev.github.io/commons/pr-436/py/ Built from the latest commit on this branch. The R links in it point at the published R site, which no pull request rebuilds. |
jat255
force-pushed
the
jat255/whhh-live-warehouse-r
branch
from
October 21, 2026 22:02
dce7070 to
ed8e840
Compare
jat255
force-pushed
the
jat255/8kf5-catalog-merge-error-classes
branch
from
October 21, 2026 22:03
2528d72 to
2e33b0e
Compare
The catalog-merge fixture names an error slug for its three failing cases, but both runners accepted any error, so an unrelated failure would pass. Each refusal now has a class (an R condition class, and a ValueError subclass in Python so existing handlers still catch it), and both runners map the slug to the class they expect.
jat255
force-pushed
the
jat255/8kf5-catalog-merge-error-classes
branch
from
October 22, 2026 02:46
2e33b0e to
97bc0e6
Compare
This branch has not been deployed
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
1 participant
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.
This PR adds an error class for each of the three ways a data dictionary can fail to match the warehouse catalog, in both packages. The shared
catalog-mergefixture already names which error each failing case expects, and both test runners now check it, so an unrelated failure no longer passes those cases.Stacked on #431.
Review notes
ValueError, so code that catchesValueErrortoday still catches them. Like the existing catalog access errors, they are exported fromcommons._catalogbut not fromcommons.ambiguous,claimed,ambiguous_column). Each runner maps a slug to its own class name.Testing
Python: ruff, pyrefly and the full pytest suite pass. R:
R CMD checkpasses with no errors, warnings or notes. It ran with_R_CHECK_FORCE_SUGGESTS_=false, becausersconnectis not installed locally. Both runners failed before the classes were added, and pass after.For the R reviewer
The three
cli_abort()calls in the dictionary merge incatalog.Rnow set a condition class:commons_catalog_ambiguous_table_error,commons_catalog_claimed_relation_errorandcommons_catalog_ambiguous_column_error. This follows the pattern of the classes incatalog-security.R. The messages and the inputs that cause an error are unchanged; the only difference is that the errors can now be caught by class.test-catalog-merge.Rnow checks the class withexpect_error(class = ), and NEWS names the classes, because these errors shipped in 0.1.1.Agent-written notes
kata: 8kf5 (milestone M10). The issue also asked for a check of the other shared fixtures for fields that no runner asserts. That audit found more, filed as kata 438r (definition-export error codes, which need production changes like this PR) and bgn3 (gaps that only one runner checks, test-only).