Skip to content

Conversation

@MonkeyCanCode
Copy link
Contributor

Description

This is a follow up PR based on #227 where additional document as well as new optional build argument (ECLIPSELINK_DEPS) for Dockerfile and run.sh.

I saw #311 already added the additional option for Dockerfile, so I ended up using that change as well.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Tested build options on Dockerfile:

# default
docker build -t localhost:5001/polaris:latest .
# with eclipse link enabled
docker build -t localhost:5001/polaris:latest --build-arg ECLIPSELINK=true .
# with eclipse link enabled and one dependency
docker build -t localhost:5001/polaris:latest --build-arg ECLIPSELINK=true --build-arg ECLIPSELINK_DEPS=org.postgresql:postgresql:42.1.4 .
# with eclipse link enabled and multiple dependencies
docker build -t localhost:5001/polaris:latest --build-arg ECLIPSELINK=true --build-arg ECLIPSELINK_DEPS=org.postgresql:postgresql:42.1.4,com.mysql:mysql-connector-j:8.1.0 .

Tested build options on run.sh:

# default
./run.sh
# with eclipse link enabled
./run.sh -b "ECLIPSELINK=true"
# with eclipse link enabled and one dependency
./run.sh -b "ECLIPSELINK=true;ECLIPSELINK_DEPS=org.postgresql:postgresql:42.1.4"
# with eclipse link enabled and multiple dependencies
./run.sh -b "ECLIPSELINK=true;ECLIPSELINK_DEPS=org.postgresql:postgresql:42.1.4,com.mysql:mysql-connector-j:8.1.0"

Checklist:

Please delete options that are not relevant.

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings

Copy link
Contributor

@aihuaxu aihuaxu left a comment

Choose a reason for hiding this comment

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

LGTM.

flyrain
flyrain previously approved these changes Sep 24, 2024
@flyrain
Copy link
Contributor

flyrain commented Sep 24, 2024

Hi @MonkeyCanCode, can you rebase for the conflict?

README.md Outdated
- `./run.sh` - To run Polaris as a mini-deployment locally. This will create one pod that bind itself to ports `8181` and `8182`.
- Optional run options:
- `./run.sh -b "ECLIPSELINK=true"` - Enables the EclipseLink extension.
- `./run.sh -b "ECLIPSELINK=true;ECLIPSELINK_DEPS=[groupId]:[artifactId]:[version],..."` – Enabled the EclipseLink extension with one or more additional dependencies for EclipseLink (e.g., JDBC drivers), separated by commas.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's semicolons not commas right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The deps for ECLIPSELINK_DEPS but the diff options are separated by commas. So following format:

-b "KEY1=V1;KEY2=V2,V3,V4;KEY3=v4"

This is because the delimiter is comma for diff optional options. Thus, I can't use comma to split deps as well.

@flyrain flyrain merged commit 3396e22 into apache:main Sep 24, 2024
@flyrain
Copy link
Contributor

flyrain commented Sep 24, 2024

Thanks @MonkeyCanCode for the PR. Thanks @eric-maynard @aihuaxu for the review.

@MonkeyCanCode MonkeyCanCode deleted the docker_eclipseLinkDeps branch December 12, 2024 04:57
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.

4 participants