Skip to content

feat: support service configuration via system properties#79

Merged
jorge-ibm merged 1 commit intomasterfrom
support-system-properties
May 11, 2020
Merged

feat: support service configuration via system properties#79
jorge-ibm merged 1 commit intomasterfrom
support-system-properties

Conversation

@jorge-ibm
Copy link
Copy Markdown
Contributor

@jorge-ibm jorge-ibm commented May 8, 2020

ref: https://github.ibm.com/arf/planning-sdk-squad/issues/1762

Changes:

  • Support java system properties as an alternative to environment variables for config properties
  • Add unit tests

files.add(new File(userSpecifiedPath));
}

if (StringUtils.isNotEmpty(userSpecifiedSystemProp)) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I couldn't move this to the end because unit tests would fail due to them finding other credential files in the java-core root directory

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah, I think this is fine. If the user sets IBM_CREDENTIALS_FILE as an env var OR a system property, i think the intent is clear that they want to specify the custom filename as an override to any of the standard names we may look for.

@jorge-ibm jorge-ibm requested a review from padamstx May 8, 2020 21:36
@jorge-ibm jorge-ibm force-pushed the support-system-properties branch from c10b00f to e4a027e Compare May 8, 2020 21:41
padamstx
padamstx previously approved these changes May 9, 2020
Copy link
Copy Markdown
Contributor

@padamstx padamstx left a comment

Choose a reason for hiding this comment

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

LGTM; I made one minor comment but I don't think it affects functionality at all.

files.add(new File(userSpecifiedPath));
}

if (StringUtils.isNotEmpty(userSpecifiedSystemProp)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yeah, I think this is fine. If the user sets IBM_CREDENTIALS_FILE as an env var OR a system property, i think the intent is clear that they want to specify the custom filename as an override to any of the standard names we may look for.

Comment thread src/main/java/com/ibm/cloud/sdk/core/util/CredentialUtils.java Outdated
Copy link
Copy Markdown
Contributor

@padamstx padamstx left a comment

Choose a reason for hiding this comment

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

LGTM

@jorge-ibm jorge-ibm merged commit d5e7a27 into master May 11, 2020
ibm-devx-automation pushed a commit that referenced this pull request May 11, 2020
# [8.2.0](8.1.5...8.2.0) (2020-05-11)

### Features

* support service config via system properties ([#79](#79)) ([d5e7a27](d5e7a27))
@ibm-devx-automation
Copy link
Copy Markdown

🎉 This PR is included in version 8.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@jorge-ibm jorge-ibm deleted the support-system-properties branch September 14, 2020 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants