Skip to content
This repository has been archived by the owner on Nov 9, 2022. It is now read-only.

Shared Config for multiple applications sharing a single content db #685

Closed
jmeekhof opened this issue Nov 2, 2016 · 11 comments
Closed

Comments

@jmeekhof
Copy link
Contributor

jmeekhof commented Nov 2, 2016

The company I work for currently has several (roxy) applications that all shared a common content-db.

This makes sharing the content database configuration difficult among all the applications difficult.

I'd like to propose the following method to handle sharing portions of configuration among multiple applications:

Create a new configuration option (living in build.properties) that would instruct roxy to read another properties file, and apply those properties after default and build, but before the environment properties file.

This option would point to a properties file outside the deploy directory. This directory would be a submodule within your project, thus allowing you to share whatever portions of the configuration you desire.

My initial investigation suggests that this check for the option would exist within server_config.rb in the
2679 def ServerConfig.properties(prop_file_location = @@path)
2680 default_properties_file = ServerConfig.expand_path("#{prop_file_location}/default.properties")
2681 properties_file = ServerConfig.expand_path("#{prop_file_location}/build.properties")
2682
2683 raise ExitException.new("You must run ml init to configure your application.") unless File.exist?(properties_file)
2684
2685 properties = ServerConfig.load_properties(default_properties_file, "ml.")
2686 properties.merge!(ServerConfig.load_properties(properties_file, "ml."))
2687 # Read properties, look for a config that would instruct us to read another optional properties file
2688 #
2689 # If that exists, then find file, and merge its properties
2690

@RobertSzkutak
Copy link
Contributor

RobertSzkutak commented Nov 2, 2016

Just double checking : You only want environment specific *.properties files to be able to live in a configurable directory, right?

@RobertSzkutak RobertSzkutak self-assigned this Nov 2, 2016
@RobertSzkutak RobertSzkutak added this to the 1.7.5 milestone Nov 2, 2016
@jmeekhof
Copy link
Contributor Author

jmeekhof commented Nov 2, 2016

I wasn't asking where, so much as describing a problem and proposing a solution.

The function referenced is where I think I'd add the logic I'm proposing.

@RobertSzkutak
Copy link
Contributor

RobertSzkutak commented Nov 2, 2016

Spoke with @jmeekhof, his proposal is as follows:

  1. Create four layers of properties files:
  1. default.properties
  2. build.properties
  3. shared.properties
  4. environment.properties

The reason for shared.properties is that he wants to store the properties within a git submodule which makes it impossible to keep them in the same directory as build.properties. Second, there may be different "groups" of projects in the future which makes this third layer desirable. Finally, you would have to specify in default.properties where build.properties should be pulled from which would defeat the purpose of never modifying default.properties.

Given this, I can't think of a way to avoid having the shared.properties.

  1. Allow for customization of where there environment.properties and shared.properties are stored. EDIT : making environment.properties configurable is not needed for his use case and wont be needed to close this issue

Seeking sign-off from @grtjn and @dmcassel as this would fundamentally shift how Roxy handles properties.

@jmeekhof
Copy link
Contributor Author

jmeekhof commented Nov 2, 2016

In my early vision of how I want to use this feature, I'm really trying to influence bootstrapping. Does bootstrapping read the properties files differently than the other commands?

@RobertSzkutak
Copy link
Contributor

RobertSzkutak commented Nov 2, 2016

@jmeekhof It shouldn't! Properties are first configured with @properties = ServerConfig.properties in ml.rb . Then initialize in server_config.rb is called with those properties previously created passed into the new ServerConfig constructor. Finally, the appropriate function for your command is called (bootstrap, deploy X, etc.). There's nothing in def bootstrap that would affect the properties as far as I can tell.

As you pointed our earlier, modifying the ServerConfig.properties function will be key to fulfilling your goals.

@jmeekhof
Copy link
Contributor Author

jmeekhof commented Nov 3, 2016

I have this change working on my fork. If others see the value, I'd love to share it.

jmeekhof pushed a commit to jmeekhof/roxy that referenced this issue Nov 3, 2016
The path indicated by shared_config looks for the file relative to the
root of the roxy project
@dmcassel
Copy link
Collaborator

dmcassel commented Nov 3, 2016

I don't mind this change as long as:

1 - we don't break projects that don't use it, meaning that the Roxy code should gracefully accept the shared.properties not existing, and
2 - it's well documented -- not just how to use it, but why someone would want to (including pointing out that this is a special case that most people can ignore). We've seen confusion about the three levels of properties, but the utility of them justifies the approach. I think we can say the same for this level, but we need to document well.

@RobertSzkutak
Copy link
Contributor

Yes, thats my take on it too. The extra layer should be completely optional. In fact, I'd even argue to not turn it on out of the box but instead to thoroughly document its niche usage scenario.

@RobertSzkutak
Copy link
Contributor

RobertSzkutak commented Nov 3, 2016

@jmeekhof Would love to see you open a PR. I have a lot of feedback for you and thats probably the best place to share it. I'm sure I will also will end up sending a PR to your PR.

@jmeekhof
Copy link
Contributor Author

jmeekhof commented Nov 3, 2016

I agree, and can provide the documentation. Currently I test for the existence of the property, making it optional.

@jmeekhof jmeekhof mentioned this issue Nov 3, 2016
@RobertSzkutak
Copy link
Contributor

Update on this issue :

Josh and I chatted back and forth the last week. In the end, he just needs an extra shared.properties layer which can have a configurable path. I updated my earlier commend to reflect his use case. PR should be ready to merge shortly and I'll tap someone else to also give it a review.

jmeekhof pushed a commit to jmeekhof/roxy that referenced this issue Nov 14, 2016
Regarding pathing to shared config file location
jmeekhof pushed a commit to jmeekhof/roxy that referenced this issue Nov 15, 2016
Description of where directory is more clear as to where the config file
is loaded from.

Fixed typo.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants