Skip to content

[MINOR] Minor fixes i.e., validation checks, formatting e.t.c.#1636

Merged
Shafaq-Siddiqi merged 1 commit intoapache:mainfrom
Shafaq-Siddiqi:release3
Jun 13, 2022
Merged

[MINOR] Minor fixes i.e., validation checks, formatting e.t.c.#1636
Shafaq-Siddiqi merged 1 commit intoapache:mainfrom
Shafaq-Siddiqi:release3

Conversation

@Shafaq-Siddiqi
Copy link
Contributor

No description provided.

@j143
Copy link
Member

j143 commented Jun 13, 2022

Hi @Shafaq-Siddiqi , can you clarify whether we need to add commons-compiler-3.0.16 to lib or this PR would be enough?

@Shafaq-Siddiqi
Copy link
Contributor Author

Hi @Shafaq-Siddiqi , can you clarify whether we need to add commons-compiler-3.0.16 to lib or this PR would be enough?

We need to add that library too along with this PR.

@Shafaq-Siddiqi Shafaq-Siddiqi merged commit fd5be66 into apache:main Jun 13, 2022
@github-pages github-pages bot temporarily deployed to github-pages June 13, 2022 16:28 Inactive
@mboehm7
Copy link
Contributor

mboehm7 commented Jun 13, 2022

Hmm, I'm still trying to understand what happened here: (1) the error comes from janino, which is provided because Spark already depends on it, but (2) we need to package all dependencies into lib of our binary artifact, and (3) it seems janino now depends on commons-compiler which needs to be added so we would need to add this to our pom file. But, why did all our previous experiments in spark drivers, and local operations never showed this? @j143 is this only because the scripts for the release process have to be updated as well?

@phaniarnab
Copy link
Contributor

We are currently not packing commons-compiler in lib for the binary. assembly/bin.xml dictates the list of libraries to be included in the binary. Even for the local experiments, we usually point to the all-included lib directory, which hides the problem. @mboehm7

@mboehm7
Copy link
Contributor

mboehm7 commented Jun 13, 2022

thanks for clarifying @phaniarnab - this makes a lot of sense.

@Baunsgaard
Copy link
Contributor

Currently our automated tests only verify that the packaged lib (from bin.xml) works in python. We just need more tests that verify execution with more varied scripts to automate this verification of packaging the minimum jar files needed.

We should also add some application test via scripts that we execute with the release without python.

- name: Run all python tests no environment

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.

5 participants