Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Kernel] Move precondition checks from DefaultKernelUtils to Preconditions class #2169

Merged
merged 11 commits into from
Oct 26, 2023

Conversation

rpinzon
Copy link
Contributor

@rpinzon rpinzon commented Oct 12, 2023

Which Delta project/connector is this regarding?

  • Spark
  • Standalone
  • Flink
  • Kernel
  • Other (fill in here)

Description

Moves checkArgument methods from DefautKernelUtils to Preconditions class to clean up the code.

Resolves #2148

How was this patch tested?

Existing tests suffice.

Does this PR introduce any user-facing changes?

No

Copy link
Collaborator

@vkorukanti vkorukanti left a comment

Choose a reason for hiding this comment

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

lgtm, just one minor comment. Thank you for your contribution.

Signed-off-by: Renan Tomazoni Pinzon <[email protected]>
Signed-off-by: Renan Tomazoni Pinzon <[email protected]>
@rpinzon
Copy link
Contributor Author

rpinzon commented Oct 17, 2023

@vkorukanti since I synced the upstream changes today I can't build the project or import it into the IDE anymore. However, when I push to my branch the pipeline runs fine.

I double checked the building instructions but it doesn't appear to have changed.

Looking at the build log, I can see that all errors are related to missing classes/objects in iceberg. (see the log)

I tried to clone and build (build/sbt compile) the upstream (master) but I faced the same error.

Am I missing something?

@vkorukanti
Copy link
Collaborator

@vkorukanti since I synced the upstream changes today I can't build the project or import it into the IDE anymore. However, when I push to my branch the pipeline runs fine.

I double checked the building instructions but it doesn't appear to have changed.

Looking at the build log, I can see that all errors are related to missing classes/objects in iceberg. (see the log)

I tried to clone and build (build/sbt compile) the upstream (master) but I faced the same error.

Am I missing something?

I just noticed the master build is broken. Made a PR. Hopefully that should fix it, let me know if not.

@rpinzon
Copy link
Contributor Author

rpinzon commented Oct 20, 2023

@vkorukanti since I synced the upstream changes today I can't build the project or import it into the IDE anymore. However, when I push to my branch the pipeline runs fine.
I double checked the building instructions but it doesn't appear to have changed.
Looking at the build log, I can see that all errors are related to missing classes/objects in iceberg. (see the log)
I tried to clone and build (build/sbt compile) the upstream (master) but I faced the same error.
Am I missing something?

I just noticed the master build is broken. Made a PR. Hopefully that should fix it, let me know if not.

hey @vkorukanti, I'm not sure if your PR has already been merged into the master but I just tried to build the project and the error still occurs

@vkorukanti
Copy link
Collaborator

hey @vkorukanti, I'm not sure if your PR has already been merged into the master but I just tried to build the project and the error still occurs

The fix I mentioned is already landed on master. Can you try following steps to understand why the icebergShaded` classes are not found.

cd <delta - repo - directory>
rm -rf icebergShaded/lib/iceberg-*
python3 ./icebergShaded/generate_iceberg_jars.py

@rpinzon
Copy link
Contributor Author

rpinzon commented Oct 25, 2023

hey @vkorukanti, I'm not sure if your PR has already been merged into the master but I just tried to build the project and the error still occurs

The fix I mentioned is already landed on master. Can you try following steps to understand why the icebergShaded` classes are not found.

cd <delta - repo - directory>
rm -rf icebergShaded/lib/iceberg-*
python3 ./icebergShaded/generate_iceberg_jars.py

Following these steps I was able to find out that the JARs were not being generated in the icebergShaded/lib directory due to a configuration in my Gradle setup. For some reason the error is printed but the build does not fail.

Thank you very much for your help @vkorukanti.

Copy link
Collaborator

@vkorukanti vkorukanti left a comment

Choose a reason for hiding this comment

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

lgtm

@vkorukanti vkorukanti merged commit 3a494e9 into delta-io:master Oct 26, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Kernel] Move precondition checks from DefaultKernelUtils to a separate pre-conditions class
2 participants