Initial Artifactory Pro plan #856
Conversation
Eventually I would like to expose more configuration data (e.g. initial users, repos etc) but this gets the job done for now! Signed-off-by: Seth Chisamore <[email protected]>
|
By analyzing the blame information on this pull request, we identified @adamhjk to be a potential reviewer |
| export JAVA_HOME=$(hab pkg path core/server-jre) | ||
| export ARTIFACTORY_HOME={{pkg.svc_var_path}} | ||
|
|
||
| {{pkg.svc_var_path}}/bin/artifactory.sh 2>&1 |
There was a problem hiding this comment.
FYI I saw @adamhjk switched the redis plan from using a run hook to a line in plan.sh:
pkg_svc_run="bin/redis-server $pkg_svc_config_path/redis.config"Maybe that's cleaner than using a run hook? Thoughts?
There was a problem hiding this comment.
@kevindickerson I think we need the hook as those exported environment variables are required in order for artifactory.sh to operate correctly. JAVA_HOME needs to be dynamically resolved at hook execution time also.
There was a problem hiding this comment.
So like
pkg_svc_run="JAVA_HOME=$(pkg_path_for core/server-jre) ARTIFACTORY_HOME=$pkg_svc_var_path $pkg_svc_var_path/bin/artifactory.sh 2>&1"
wouldn't work? (I think pkg_path_for should pull it at execute time??)
There was a problem hiding this comment.
Things begin to get pretty unreadable at some point. I would love to hear a comparison on using pkg_svc_run vs a run hook. @fnichol ?
There was a problem hiding this comment.
I'm honestly not sure which is better than the other. Certainly having a separate run hook is less long. :)
There was a problem hiding this comment.
Oh, also worth noting that pkg_svc_run should not fork! Also ends with 2>&1 by default
There was a problem hiding this comment.
I'd say you should make a run hook if the thing gets longer that what should fit on one line, as a loose, subjective, guideline.
pkg_svc_run is nice for plans that don't have any other hooks because you only need to have the plan.sh file, but if you already have a hooks directory, and your run hook is more than one line, put it in a separate hook.
|
📌 Commit c13a4d7 has been approved by |
Eventually I would like to expose more configuration data (e.g. initial users, repos etc) but this gets the job done for now! Signed-off-by: Seth Chisamore <[email protected]> Pull request: #856 Approved by: smith
|
☀️ Test successful - travis |
Eventually I would like to expose more configuration data (e.g. initial users, repos etc) but this gets the job done for now!
Signed-off-by: Seth Chisamore [email protected]