-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
ObjectStoreEnvironmentBean.getObjectStore should not cache the user.home during build-time #6555
Conversation
…ome during build-time Fixes #6553
*/ | ||
@Substitute | ||
public String getObjectStoreDir() { | ||
return System.getProperty("user.home") + File.separator + "ObjectStore"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure this won't be cache this at build time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you afraid that the System.getProperty("user.home")
is cached at build time?
Yes. Try running the quickstarts with this PR and you'll see the tests pass
Em Ter, 14 de jan de 2020 15:23, Clement Escoffier <notifications@github.com>
escreveu:
… ***@***.**** commented on this pull request.
------------------------------
In
extensions/narayana-stm/runtime/src/main/java/io/quarkus/narayana/stm/runtime/ObjectStoreEnvironmentBeanSubstitution.java
<#6555 (comment)>:
> +package io.quarkus.narayana.stm.runtime;
+
+import java.io.File;
+
+import com.oracle.svm.core.annotate.Substitute;
+import com.oracle.svm.core.annotate.TargetClass;
+
***@***.***(className = "com.arjuna.ats.arjuna.common.ObjectStoreEnvironmentBean")
+public final class ObjectStoreEnvironmentBeanSubstitution {
+
+ /**
+ * @return fixed ObjectStore path resolved during runtime
+ */
+ @substitute
+ public String getObjectStoreDir() {
+ return System.getProperty("user.home") + File.separator + "ObjectStore";
Are we sure this won't be cache this at build time?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6555?email_source=notifications&email_token=AAANG5LBZCHXVZWYBITS36LQ5X7JFA5CNFSM4KGW4I72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCRW5IRI#pullrequestreview-342742085>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAANG5NPYRHUF4CQKHL6CHDQ5X7JFANCNFSM4KGW4I7Q>
.
|
Afaik It is cached because the objectstore path is set in the constructor.
See the stacktrace in the issue
Em Qua, 15 de jan de 2020 03:33, Georgios Andrianakis <
notifications@github.com> escreveu:
… ***@***.**** commented on this pull request.
------------------------------
In
extensions/narayana-stm/runtime/src/main/java/io/quarkus/narayana/stm/runtime/ObjectStoreEnvironmentBeanSubstitution.java
<#6555 (comment)>:
> +package io.quarkus.narayana.stm.runtime;
+
+import java.io.File;
+
+import com.oracle.svm.core.annotate.Substitute;
+import com.oracle.svm.core.annotate.TargetClass;
+
***@***.***(className = "com.arjuna.ats.arjuna.common.ObjectStoreEnvironmentBean")
+public final class ObjectStoreEnvironmentBeanSubstitution {
+
+ /**
+ * @return fixed ObjectStore path resolved during runtime
+ */
+ @substitute
+ public String getObjectStoreDir() {
+ return System.getProperty("user.home") + File.separator + "ObjectStore";
Are you afraid that the System.getProperty("user.home") is cached at
build time?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#6555?email_source=notifications&email_token=AAANG5J7Y2CSQTOJCKA7E7TQ52U2RA5CNFSM4KGW4I72YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCRY6XMY#discussion_r366713592>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAANG5JSZE76J63LMXA6SATQ52U2RANCNFSM4KGW4I7Q>
.
|
@gastaldi I assume you tried the STM quickstart with this fix? |
Of course, otherwise I wouldn't even bother opening it 😃 |
Just making sure :) |
@cescoffier up to you 😉 |
With this merged we can re-enable the STM module in the quickstart CI thingy quarkusio/quarkus-quickstarts#420 |
Fixes #6553