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

Resolve multiple environment variables in configuration values #75

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

dnskr
Copy link

@dnskr dnskr commented Apr 6, 2024

Description

The PR adds support for multiple environment variables in configuration values.
For example, the following value is valid if $HOSTNAME and $USERNAME environment variables defined:

node.id=prefix-${ENV:HOSTNAME}-${ENV:USERNAME}-suffix

The change is continuation #63 and similar to airlift#872.

Test

mvn -f .\bootstrap\pom.xml test
[INFO] Scanning for projects...
[INFO] 
[INFO] -------------------< com.facebook.airlift:bootstrap >-------------------
[INFO] Building bootstrap 0.211-SNAPSHOT
[INFO]   from pom.xml
[INFO] --------------------------------[ jar ]---------------------------------

...

[INFO] Tests run: 21, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 2.015 s - in TestSuite
[INFO] 
[INFO] Results:
[INFO]
[INFO] Tests run: 21, Failures: 0, Errors: 0, Skipped: 0
[INFO]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  8.835 s
[INFO] Finished at: 2024-04-06T18:47:47+02:00
[INFO] ------------------------------------------------------------------------

@tdcmeehan tdcmeehan self-assigned this Apr 16, 2024
Copy link

@tdcmeehan tdcmeehan left a comment

Choose a reason for hiding this comment

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

Were you able to build this and run mvn:verify? Locally I found a build issue in the stats module, did you encounter that as well?

@rschlussel
Copy link

Were you able to build this and run mvn:verify? Locally I found a build issue in the stats module, did you encounter that as well?

Wow we should set up CI for this. We have no guarantee now that build and tests pass for new PRs.

@tdcmeehan
Copy link

Agreed (see #64).

@dnskr
Copy link
Author

dnskr commented Apr 17, 2024

Were you able to build this and run mvn:verify? Locally I found a build issue in the stats module, did you encounter that as well?

@tdcmeehan It doesn't work with Java 11 which probably you use locally, but it works if Java 8 is used.

Broken for Java 11:

$ export JAVA_HOME='C:\Users\dnskr\.jdks\temurin-11.0.22' && mvn verify -DskipTests

[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------

...

[INFO] -------------------------------------------------------------
[ERROR] COMPILATION ERROR :
[INFO] -------------------------------------------------------------
[ERROR] C:\Users\dnskr\IdeaProjects\airlift\stats\src\main\java\com\facebook\airlift\stats\JmxGcMonitor.java:[21,23] error: cannot find symbol
  symbol:   class PostConstruct
  location: package javax.annotation
[ERROR] C:\Users\dnskr\IdeaProjects\airlift\stats\src\main\java\com\facebook\airlift\stats\JmxGcMonitor.java:[22,23] error: cannot find symbol
  symbol:   class PreDestroy
  location: package javax.annotation
[ERROR] C:\Users\dnskr\IdeaProjects\airlift\stats\src\main\java\com\facebook\airlift\stats\PauseMeter.java:[24,23] error: cannot find symbol
  symbol:   class PostConstruct
  location: package javax.annotation
[ERROR] C:\Users\dnskr\IdeaProjects\airlift\stats\src\main\java\com\facebook\airlift\stats\PauseMeter.java:[25,23] error: cannot find symbol
  symbol:   class PreDestroy
  location: package javax.annotation
[ERROR] C:\Users\dnskr\IdeaProjects\airlift\stats\src\main\java\com\facebook\airlift\stats\JmxGcMonitor.java:[62,5] error: cannot find symbol
  symbol:   class PostConstruct
  location: class JmxGcMonitor
[ERROR] C:\Users\dnskr\IdeaProjects\airlift\stats\src\main\java\com\facebook\airlift\stats\JmxGcMonitor.java:[80,5] error: cannot find symbol
  symbol:   class PreDestroy
  location: class JmxGcMonitor
[ERROR] C:\Users\dnskr\IdeaProjects\airlift\stats\src\main\java\com\facebook\airlift\stats\PauseMeter.java:[63,5] error: cannot find symbol
  symbol:   class PostConstruct
  location: class PauseMeter
[ERROR] C:\Users\dnskr\IdeaProjects\airlift\stats\src\main\java\com\facebook\airlift\stats\PauseMeter.java:[69,5] error: cannot find symbol
  symbol:   class PreDestroy
  location: class PauseMeter
[INFO] 8 errors
[INFO] -------------------------------------------------------------
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for airlift 0.211-SNAPSHOT:
[INFO]
[INFO] airlift ............................................ SUCCESS [  4.828 s]
[INFO] log ................................................ SUCCESS [  2.067 s]
[INFO] testing ............................................ SUCCESS [  1.006 s]
[INFO] configuration ...................................... SUCCESS [  1.215 s]
[INFO] log-manager ........................................ SUCCESS [  0.974 s]
[INFO] bootstrap .......................................... SUCCESS [  0.867 s]
[INFO] concurrent ......................................... SUCCESS [  0.813 s]
[INFO] http-utils ......................................... SUCCESS [  0.598 s]
[INFO] json ............................................... SUCCESS [  1.085 s]
[INFO] stats .............................................. FAILURE [  2.861 s]
[INFO] security ........................................... SKIPPED
[INFO] trace-token ........................................ SKIPPED
[INFO] http-client ........................................ SKIPPED
[INFO] node ............................................... SKIPPED
[INFO] discovery .......................................... SKIPPED
[INFO] dbpool ............................................. SKIPPED
[INFO] event .............................................. SKIPPED
[INFO] http-server ........................................ SKIPPED
[INFO] jaxrs .............................................. SKIPPED
[INFO] jaxrs-testing ...................................... SKIPPED
[INFO] jmx-http-rpc ....................................... SKIPPED
[INFO] jmx-http ........................................... SKIPPED
[INFO] jmx ................................................ SKIPPED
[INFO] launcher ........................................... SKIPPED
[INFO] packaging .......................................... SKIPPED
[INFO] skeleton-server .................................... SKIPPED
[INFO] sample-server ...................................... SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  16.930 s
[INFO] Finished at: 2024-04-17T20:45:48+02:00
[INFO] ------------------------------------------------------------------------

Works for Java 8:

$ export JAVA_HOME='C:\Users\dnskr\.jdks\temurin-1.8.0_382' && mvn verify -DskipTests

[INFO] Scanning for projects...
[INFO] ------------------------------------------------------------------------

...

[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for airlift 0.211-SNAPSHOT:
[INFO]
[INFO] airlift ............................................ SUCCESS [  3.723 s]
[INFO] log ................................................ SUCCESS [  1.853 s]
[INFO] testing ............................................ SUCCESS [  0.959 s]
[INFO] configuration ...................................... SUCCESS [  1.179 s]
[INFO] log-manager ........................................ SUCCESS [  1.018 s]
[INFO] bootstrap .......................................... SUCCESS [  1.063 s]
[INFO] concurrent ......................................... SUCCESS [  0.854 s]
[INFO] http-utils ......................................... SUCCESS [  0.581 s]
[INFO] json ............................................... SUCCESS [  1.048 s]
[INFO] stats .............................................. SUCCESS [  2.929 s]
[INFO] security ........................................... SUCCESS [  0.801 s]
[INFO] trace-token ........................................ SUCCESS [  0.573 s]
[INFO] http-client ........................................ SUCCESS [  1.704 s]
[INFO] node ............................................... SUCCESS [  0.725 s]
[INFO] discovery .......................................... SUCCESS [  1.283 s]
[INFO] dbpool ............................................. SUCCESS [  1.299 s]
[INFO] event .............................................. SUCCESS [  1.185 s]
[INFO] http-server ........................................ SUCCESS [  1.519 s]
[INFO] jaxrs .............................................. SUCCESS [  1.752 s]
[INFO] jaxrs-testing ...................................... SUCCESS [  1.577 s]
[INFO] jmx-http-rpc ....................................... SUCCESS [  1.125 s]
[INFO] jmx-http ........................................... SUCCESS [  1.390 s]
[INFO] jmx ................................................ SUCCESS [  1.081 s]
[INFO] launcher ........................................... SUCCESS [  0.337 s]
[INFO] packaging .......................................... SUCCESS [  0.433 s]
[INFO] skeleton-server .................................... SUCCESS [  3.211 s]
[INFO] sample-server ...................................... SUCCESS [  2.622 s]
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  38.306 s
[INFO] Finished at: 2024-04-17T20:44:15+02:00
[INFO] ------------------------------------------------------------------------

@dnskr
Copy link
Author

dnskr commented Apr 17, 2024

Created issue #79 related to failing build on Java 11.

@dnskr dnskr force-pushed the resolve-many-env-vars-in-configs branch from d54153c to 3a9f1c8 Compare April 19, 2024 21:15
@dnskr dnskr requested a review from a team as a code owner April 19, 2024 21:15
@dnskr dnskr requested review from presto-oss and tdcmeehan April 19, 2024 21:15
@tdcmeehan tdcmeehan merged commit 4fdcdb1 into prestodb:master Apr 22, 2024
2 checks passed
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.

3 participants