Add pre-publish step for IcebergSource and fix other issues in Iceberg file based copy #4155
Conversation
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
thisisArjit
force-pushed
the
oh-azure-workaround
branch
from
7acb2c0 to
8a09042
Compare
thisisArjit
force-pushed
the
oh-azure-workaround
branch
4 times, most recently
from
a6e6ef9 to
1d2fa8b
Compare
thisisArjit
force-pushed
the
oh-azure-workaround
branch
2 times, most recently
from
73a7ba0 to
883ad29
Compare
thisisArjit
changed the title
thisisArjit
force-pushed
the
oh-azure-workaround
branch
from
883ad29 to
ed667f2
Compare
thisisArjit
changed the title
thisisArjit
force-pushed
the
oh-azure-workaround
branch
from
ed667f2 to
705ea0a
Compare
| /** | ||
| * Invoke the private addDeleteStepIfNeeded method using reflection | ||
| */ | ||
| private void invokeAddDeleteStepIfNeeded(List<IcebergTable.FilePathWithPartition> sourceFiles, |
Contributor
There was a problem hiding this comment.
sourceFiles is not being used while invoking the method addDeleteStepIfNeeded
Comment on lines
355
to
359
| List<IcebergTable.FilePathWithPartition> sourceFiles = Lists.newArrayList(); | ||
| Map<String, String> partitionData = new HashMap<>(); | ||
| partitionData.put("datepartition", "2025-10-11"); | ||
| sourceFiles.add(new IcebergTable.FilePathWithPartition("/source/path/file1.parquet", partitionData, 1000L)); | ||
| sourceFiles.add(new IcebergTable.FilePathWithPartition("/source/path/file4.parquet", partitionData, 1000L)); |
Contributor
There was a problem hiding this comment.
sourceFiles is not being used in invokeDeleteStepIfNeeded, this test is effectively verifying similar behaviour as testDeleteSinglePartitionDirectory
| public void testDeleteTargetDirectoryNotExists() throws Exception { | ||
| // Configure: delete enabled but point to non-existent directory | ||
| state.setProp(IcebergSource.DELETE_FILES_NOT_IN_SOURCE, true); | ||
| state.setProp(ConfigurationKeys.DATA_PUBLISHER_FINAL_DIR, "/non/existent/path"); |
Contributor
There was a problem hiding this comment.
can also add a check for testing when DATA_PUBLISHER_FINAL_DIR is not set i.e. before setting this property in this test itself, and rename to something like testDeleteTargetDirectoryNotConfiguredOrNotExists
| .datasetOutputPath(targetFs.getUri().getPath()) | ||
| .ancestorsOwnerAndPermission(ancestorOwnerAndPermissionList) | ||
| .build(); | ||
| copyableFile.setFsDatasets(originFs, targetFs); |
Contributor
There was a problem hiding this comment.
let's also add a test that verifies that after extraction if we deserialize the CopyableFile then destinationData is not null, hence, also test the lineage fix
thisisArjit
force-pushed
the
oh-azure-workaround
branch
2 times, most recently
from
c21d238 to
d66598e
Compare
Contributor
|
LGTM! |
khandelwal-prateek
approved these changes
Nov 24, 2025
thisisArjit
force-pushed
the
oh-azure-workaround
branch
from
d66598e to
70fe114
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
JIRA
Description
stream closed exceptionduring process work unit stepFileAwareInputStreamDataWriter can only process one file and cannot be reusedTests
Commits