Skip to content

Conversation

@Anilople
Copy link
Contributor

Describe what this PR does / why we need it

${user.home} in application.properties should be resolved in dashboard runtime, not after maven package.

Does this pull request fix one issue?

Fixes #2045

Describe how you did it

Change maven-resources-plugin's behavior by use delimiter @

Describe how to verify it

Run mvn clean package -DskipTests on project, then check BOOT-INF/classes/application.properties in file sentinel-dashboard/target/sentinel-dashboard.jar

application.properties's content is changed.

Only

sentinel.dashboard.version=@project.version@

is changed to

sentinel.dashboard.version=1.8.2-SNAPSHOT

And

logging.file=${user.home}/logs/csp/sentinel-dashboard.log

keep same as source code.

Special notes for reviews

${user.home} should be resolve in dashboard runtime, not maven package.
@CLAassistant
Copy link

CLAassistant commented Feb 24, 2021

CLA assistant check
All committers have signed the CLA.

@cdfive cdfive added area/dashboard Issues or PRs about Sentinel Dashboard kind/enhancement Category issues or prs related to enhancement. labels Feb 24, 2021
@sczyh30 sczyh30 added this to the 1.8.2 milestone Feb 24, 2021
Copy link
Collaborator

@cdfive cdfive left a comment

Choose a reason for hiding this comment

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

LGTM.
According the document the issue referred, the default delimiters are,

<delimiters>
  <delimiter>${*}</delimiter>
  <delimiter>@</delimiter>
</delimiters>

So here need to specify the @ as the only delimiter.
I package it, and get the expected application.properties in the fat jar.
Run the jar, get the expected log file.
It works fine and must be very helpful for users, thanks! @Anilople

@sczyh30 sczyh30 merged commit d67d842 into alibaba:master Feb 25, 2021
@sczyh30
Copy link
Member

sczyh30 commented Feb 25, 2021

Thanks for contributing!

@Anilople Anilople deleted the enhancement/dashboard/config/file/resolve/20210224 branch April 12, 2021 15:12
hughpearse pushed a commit to hughpearse/Sentinel that referenced this pull request Jun 2, 2021
… dashboard pom (alibaba#2046)

`${user.home}` should be resolve in dashboard runtime, not in maven package goal
linkolen pushed a commit to shivagowda/Sentinel that referenced this pull request Aug 3, 2021
… dashboard pom (alibaba#2046)

`${user.home}` should be resolve in dashboard runtime, not in maven package goal
linkolen pushed a commit to shivagowda/Sentinel that referenced this pull request Aug 14, 2021
… dashboard pom (alibaba#2046)

`${user.home}` should be resolve in dashboard runtime, not in maven package goal
linkolen pushed a commit to shivagowda/Sentinel that referenced this pull request Aug 14, 2021
… dashboard pom (alibaba#2046)

`${user.home}` should be resolve in dashboard runtime, not in maven package goal
linkolen pushed a commit to shivagowda/Sentinel that referenced this pull request Aug 15, 2021
… dashboard pom (alibaba#2046)

`${user.home}` should be resolve in dashboard runtime, not in maven package goal
linkolen pushed a commit to shivagowda/Sentinel that referenced this pull request Aug 16, 2021
… dashboard pom (alibaba#2046)

`${user.home}` should be resolve in dashboard runtime, not in maven package goal
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/dashboard Issues or PRs about Sentinel Dashboard kind/enhancement Category issues or prs related to enhancement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dashboard config logging.file's value should not be resolved after maven package

4 participants