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

Comment (nearly) all properties in properties files #138

Merged
merged 4 commits into from
Mar 20, 2019
Merged

Comment (nearly) all properties in properties files #138

merged 4 commits into from
Mar 20, 2019

Conversation

severo
Copy link
Contributor

@severo severo commented Feb 25, 2019

Since a default value is set in the geOrchestra code when any property
is loaded from a properties file, it's not necessary to provide it also
in the datadir properties files, unless it is different from the default.

It also makes it easier for the instance administrator to review their
datadir, focusing only on important properties, and letting the default
values unchanged for the rest of the properties.

See #129 for the
motivation.

Note that the default.properties file has been let uncommented, as we
always want these values to be set by the instance administrator.

@severo
Copy link
Contributor Author

severo commented Feb 25, 2019

Fixes #129

Please review with care (with the work on #2496, all properties should use their default value in case the property had not been set in datadir, but...)

@severo severo changed the title Comment (nearly) all properties in preperties files Comment (nearly) all properties in properties files Feb 25, 2019
@landryb
Copy link
Member

landryb commented Feb 26, 2019

I like it, it will clearly show what has been modified from the defaults - even if i'll have to remerge all the config changes for the ansible playbook, it'll be nicer in the end :)

@severo
Copy link
Contributor Author

severo commented Feb 26, 2019

Excellent... It's good to have the feedback from you since you are managing an instance... As I wrote in the issue, I had the same appreciation when I managed one, it should be easier for the administrators.

@fvanderbiest
Copy link
Member

Wow, this is really excellent Sylvain.
Thanks a lot.

Please rebase before merge.

Since a default value is set in the geOrchestra code when any property
is loaded from a properties file, it's not necessary to provide it also
in the datadir properties files, unless it is different from the default.

It also makes it easier for the instance administrator to review their
datadir, focusing only on important properties, and letting the default
values unchanged for the rest of the properties.

See #129 for the
motivation.

Note that the default.properties file has been let uncommented, as we
always want these values to be set by the instance administrator.
@severo
Copy link
Contributor Author

severo commented Mar 20, 2019

There is some more work due to the commits meanwhile.

@severo
Copy link
Contributor Author

severo commented Mar 20, 2019

@fvanderbiest fvanderbiest merged commit 4b94aaf into georchestra:18.12 Mar 20, 2019
@severo severo deleted the comment-applications-properties-files branch March 20, 2019 13:10
fvanderbiest pushed a commit to georchestra/extractorapp that referenced this pull request Apr 2, 2023
It's required because all the application properties will be commented
by default in the datadir. See
georchestra/datadir#137 and
georchestra/datadir#138.
fvanderbiest pushed a commit to georchestra/atlas that referenced this pull request Apr 2, 2023
It's required because all the application properties will be commented
by default in the datadir. See
georchestra/datadir#137 and
georchestra/datadir#138.
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