Repository navigation
Conversation
…ith _ extract_partition_pager ran is_hidden on the full object path, so every object of a table at a path like _staging/ev was skipped and the symlink exporter wrote one empty symlink.txt. Check only the part of the path below the table path, as table_extractor.lua and Spark do. Closes treeverse#10584
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
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.
Closes #10584
Change Description
Background
table_extractor.luaalready checks hidden segments only below_lakefs_tables/, and Spark does the same when it lists a table (HadoopFSUtils.shouldFilterOutPathlooks only at components below the base path). The Hive partition pager did not.Bug Fix
_staging/evwrites one emptyev//symlink.txtand the run reports success. The same table atstaging/evexports onesymlink.txtper partition.extract_partition_pagerinpkg/actions/lua/lakefs/catalogexport/hive.luacallspathlib.is_hidden(entry.path)on the full repository path.is_hiddenis true when any segment starts with_, so every object under_staging/evis skipped and the pager returns a single empty partition./-terminated prefix once, and runis_hiddenonly on the part of each path below it. An object exactly at the table path counts as"". Paths outside the prefix (for example a sibling_staging/ev2/...returned by the prefix listing) keep the full-path check._SUCCESS,_temporary/and other_entries below the table path are still skipped.IsHiddenis unchanged. Onlyhive.luachanges; the symlink and Glue exporters use it as is.New Feature
N/A
Testing Details
New Lua fixture
catalogexport_hive_partition_pager_hidden_baseinTestLuaRunTable. Its cases:_staging/evwith and without a trailing slash: two partitions, with_SUCCESS,_tmp/and_temporary/below the base, a.crcfile, and a sibling_staging/ev2""_staging/raw, including an object exactly at the table patha=_foo_aOn master every
_base yields one empty partition. With the fix the expected partitions and files come back. The existingcatalogexport_hive_partition_pagergolden file is unchanged.Manual: I ran the issue's repro on
lakefs run --quickstartbuilt from this branch. The hook's writer also PUTs through the lakeFS S3 gateway, and the data includes_SUCCESSand_temporary/0/part-9.parquetbelow the table path.ev//symlink.txt.ev/dt=2024-01-01/symlink.txtwith one physical address andev/dt=2024-01-02/symlink.txtwith two. The hidden files are not listed.Breaking Change?
No. Only tables whose path has a segment starting with
_behave differently, and those exported nothing before. For other tables the check below the base gives the same result as the old full-path check.Additional info
Out of scope, unchanged:
_(for example_a=1) are still skipped, as in Spark.<name>//symlink.txt, and the run still reports success. Whether that should fail is a separate decision.examples/hooks/s3_hive_manifest_exporter.lua(lines 115 and 167) has the same full-pathis_hiddencheck. It is an example hook, not the shipped exporter, so it is not changed here. I can include it if you prefer.Contact Details
Comments on this PR.
Drafted-by: Claude Opus 5.5; reviewed by @Vamsi-klu before posting