Skip to content

Remove unnecessary directory checks in delta lake#16200

Merged
findepi merged 2 commits intotrinodb:masterfrom
pajaks:pajaks/remove_hdfs_filesytem
Mar 6, 2023
Merged

Remove unnecessary directory checks in delta lake#16200
findepi merged 2 commits intotrinodb:masterfrom
pajaks:pajaks/remove_hdfs_filesytem

Conversation

@pajaks
Copy link
Copy Markdown
Member

@pajaks pajaks commented Feb 21, 2023

Description

Origin of this change is to migrate to TrinoFileSystem
Relates to #16020

Additional context and related issues

Release notes

(x ) This is not user-visible or docs only and no release notes are required.
( ) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Feb 21, 2023
@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Feb 21, 2023

/test-with-secrets sha=6866240318db76360cebff85c9ca0432f9251395

@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4233580804

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check can be skipped because directory is created implicitly when first file is created.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be part of the commit message

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be redundant check, as individual files (and implicitly directories) are created at this point. If location would be file it should fail during directory creation earlier.

@pajaks pajaks changed the title Remove unnecessary directory checks Remove unnecessary directory checks in delta lake Feb 22, 2023
@pajaks pajaks marked this pull request as ready for review February 22, 2023 10:14
@pajaks pajaks added the delta-lake Delta Lake connector label Feb 22, 2023
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be part of the commit message

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Mar 3, 2023

nit: I would include "in Delta Lake" or something to commit titles.

Ensuring that path exists can be skipped because directory is created implicitly when first file is created.
@pajaks
Copy link
Copy Markdown
Member Author

pajaks commented Mar 3, 2023

Commit message changed.

@pajaks
Copy link
Copy Markdown
Member Author

pajaks commented Mar 6, 2023

@ebyhr @findepi Could you run tests with secretes?

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Mar 6, 2023

@pajaks Was there any change since last review except for the commit title? We don't need to run CI if the change is only commit title.

@pajaks
Copy link
Copy Markdown
Member Author

pajaks commented Mar 6, 2023

@pajaks Was there any change since last review except for the commit title? We don't need to run CI if the change is only commit title.

Only commit title changes since last review, but there was other changes since last run with secret.

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Mar 6, 2023

/test-with-secrets sha=ddf985373d9fcabbe8299798b453526ef69de737

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 6, 2023

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4342318738

@findepi
Copy link
Copy Markdown
Member

findepi commented Mar 6, 2023

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/4342318738

Error: ] Could not find the selected project in the reactor: :trino-ignite @ 
Error:  Could not find the selected project in the reactor: :trino-ignite -> [Help 1]
Error:  
Error:  To see the full stack trace of the errors, re-run Maven with the -e switch.
Error:  Re-run Maven using the -X switch to enable full debug logging.
Error:  
Error:  For more information about the errors and possible solutions, please read the following articles:
Error:  [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MavenExecutionException
Error: Process completed with exit code 1.

this means that test-other-modules didn't work at all.
but these don't use secrets, so proceeding to merge

@findepi findepi merged commit 2ddb5d8 into trinodb:master Mar 6, 2023
@findepi findepi added the no-release-notes This pull request does not require release notes entry label Mar 6, 2023
@github-actions github-actions bot added this to the 410 milestone Mar 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed delta-lake Delta Lake connector no-release-notes This pull request does not require release notes entry

Development

Successfully merging this pull request may close these issues.

4 participants