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

[GAE-Java] Make GAE Java SDK version align with GraphScope #2077

Merged
merged 3 commits into from
Sep 27, 2022

Conversation

zhanglei1949
Copy link
Collaborator

What do these changes do?

Related issue number

Make GAE Java SDK version not fixed in code/configuration. BUT we still need to change analytical_engine/java/pom.xml for GraphScope version change.

@zhanglei1949 zhanglei1949 changed the title [GAE-Java] Make GAE Java SDK version consist with GAE [GAE-Java] Make GAE Java SDK version align with GAE Sep 26, 2022
@zhanglei1949 zhanglei1949 changed the title [GAE-Java] Make GAE Java SDK version align with GAE [GAE-Java] Make GAE Java SDK version align with GraphScope Sep 26, 2022
GRAPE_PROCESSOR_JAR = os.path.join(
GRAPHSCOPE_HOME, "lib", "grape-runtime-0.16.0-shaded.jar"
)
GRAPE_PROCESSOR_JAR = glob.glob(GRAPHSCOPE_HOME + "/lib/grape-runtime*.jar")[0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This [0] seems dangerous...
Could you handle the exception incase the jar is not there? So we won't crash the coordinator.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, I have changed to

ANALYTICAL_ENGINE_JAVA_RUNTIME_JAR = os.path.join(
    ANALYTICAL_ENGINE_JAVA_HOME,
    "lib",
    "grape-runtime-{}-shaded.jar".format(__version__),
)

where the __version__ is imported from version.py


ANALYTICAL_ENGINE_JAVA_RUNTIME_JAR = glob.glob(
ANALYTICAL_ENGINE_JAVA_HOME + "/lib/grape-runtime*.jar"
)[0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

And this,

)[0]
ANALYTICAL_ENGINE_JAVA_GRAPHX_JAR = glob.glob(
ANALYTICAL_ENGINE_JAVA_HOME + "/lib/graphx-on-graphscope*.jar"
)[0]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This

@codecov-commenter
Copy link

codecov-commenter commented Sep 27, 2022

Codecov Report

Merging #2077 (652b1c5) into main (652b1c5) will not change coverage.
The diff coverage is n/a.

❗ Current head 652b1c5 differs from pull request most recent head 1e23c3d. Consider uploading reports for the commit 1e23c3d to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2077   +/-   ##
=======================================
  Coverage   72.89%   72.89%           
=======================================
  Files          89       89           
  Lines        9872     9872           
=======================================
  Hits         7196     7196           
  Misses       2676     2676           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 652b1c5...1e23c3d. Read the comment docs.

organize gae java version

format

fix
@lidongze0629 lidongze0629 merged commit 1846e90 into alibaba:main Sep 27, 2022
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.

None yet

5 participants