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

[MINOR] Fix deployment issues #4749

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jongyoul
Copy link
Member

@jongyoul jongyoul commented Apr 3, 2024

What is this PR for?

Fixing some issues that happened when releasing.

What type of PR is it?

Hot Fix

Todos

  • - Remove some plugins from the shell interpreter instead of removing them from the distribution as some interpreters depend on it
  • - Upgrade python version for docker image to 3.9

What is the Jira issue?

N/A

How should this be tested?

Distribution and building docker image should work fine.

Screenshots (if appropriate)

Questions:

  • Does the license files need to update? No
  • Is there breaking changes for older versions? No
  • Does this needs documentation? No

</plugin>
<plugin>
<artifactId>maven-shade-plugin</artifactId>
<configuration>
Copy link
Member

Choose a reason for hiding this comment

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

what's the effect of this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

When publishing all interpreters including shell, maven-shade-plugin makes a fatjar and copies it into interpreter directory. maven-resource-plugin works similarly except for copying configurations to the directory.

FYI,

<!-- generate interpreter shade jar and put it under folder interpreter/${interpreter.name}-->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-shade-plugin</artifactId>
<configuration>
<createDependencyReducedPom>false</createDependencyReducedPom>
<filters>
<filter>
<artifact>*:*</artifact>
<excludes>
<exclude>META-INF/*.SF</exclude>
<exclude>META-INF/*.DSA</exclude>
<exclude>META-INF/*.RSA</exclude>
<exclude>org.apache.zeppelin:zeppelin-interpreter-shaded</exclude>
</excludes>
</filter>
</filters>
<transformers>
<transformer implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer" />
<transformer implementation="org.apache.maven.plugins.shade.resource.AppendingTransformer">
<resource>reference.conf</resource>
</transformer>
</transformers>
<artifactSet>
<excludes>
<exclude>org.apache.zeppelin:zeppelin-interpreter-shaded</exclude>
</excludes>
</artifactSet>
<outputFile>${project.basedir}/../interpreter/${interpreter.name}/${project.artifactId}-${project.version}.jar</outputFile>
</configuration>
<executions>
<execution>
<phase>package</phase>
<goals>
<goal>shade</goal>
</goals>
</execution>
</executions>
</plugin>
<plugin>
<artifactId>maven-resources-plugin</artifactId>
<executions>
<execution>
<id>copy-interpreter-setting</id>
<phase>package</phase>
<goals>
<goal>resources</goal>
</goals>
<configuration>
<outputDirectory>${project.build.directory}/../../interpreter/${interpreter.name}</outputDirectory>
</configuration>
</execution>
</executions>
</plugin>

Copy link
Member

Choose a reason for hiding this comment

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

It seems the whole sh dir disappears in the interpreter folder, not only "configurations", is it expected?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, correct. Shell interpreter was and will be removed from the official binary because it lead a lot of security issues.

Copy link
Member

@pan3793 pan3793 Apr 3, 2024

Choose a reason for hiding this comment

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

LGTM if it's the expected behavior. Test may need to be adjusted to adapt this change.

@jongyoul
Copy link
Member Author

jongyoul commented Apr 3, 2024

There are a lot of test failures. Let me fix it one by one soon

@jongyoul jongyoul marked this pull request as draft April 3, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants