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

load config from System Properties if present #96

Merged
merged 9 commits into from
May 3, 2023

Conversation

JayaGupta-Bicycle
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented May 2, 2023

Codecov Report

Merging #96 (a9a93c8) into master (02848db) will increase coverage by 0.34%.
The diff coverage is 95.45%.

❗ Current head a9a93c8 differs from pull request most recent head 7361445. Consider uploading reports for the commit 7361445 to get more accurate results

@@             Coverage Diff              @@
##             master      #96      +/-   ##
============================================
+ Coverage     85.69%   86.03%   +0.34%     
- Complexity      415      425      +10     
============================================
  Files            40       40              
  Lines          2076     2098      +22     
  Branches        194      201       +7     
============================================
+ Hits           1779     1805      +26     
+ Misses          204      199       -5     
- Partials         93       94       +1     
Impacted Files Coverage Δ
...ava/ai/apptuit/metrics/jinsight/ConfigService.java 80.93% <95.45%> (+4.24%) ⬆️

if(propertyValue == null || propertyValue.equals("")){
propertyValue = System.getProperty(propertyName);
}
return propertyValue;
Copy link
Contributor

Choose a reason for hiding this comment

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

If propertyValue is "" return null, so that we dont have to do !="" check everywhere

return config;
}

private static Map<String, Object> loadSystemProperties() {
Map<String, Object> systemProperties = new HashMap<>();
String reporter = getProperty(REPORTER_PROPERTY_NAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

System prop name should be jinsight.reporter (even though the property name is just reporter)

systemProperties.put(API_ENDPOINT_PROPERTY_NAME, apiEndpoint);
}

String globalTags = getProperty(GLOBAL_TAGS_PROPERTY_NAME);
Copy link
Contributor

Choose a reason for hiding this comment

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

System prop name should be jinsight.global_tags (even though the property name is just global_tags)

}

private static String getProperty(String propertyName) {
String propertyValue = System.getenv(propertyName);
Copy link
Contributor

Choose a reason for hiding this comment

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

if not propertyName.contains(".") then propertyName="jinsight."+propertyName

Then all new constants GLOBAL_TAGS_SYSTEM_PROPERTY_NAME REPORTER_SYSTEM_PROPERTY_NAME etc can be deleted

@JayaGupta-Bicycle JayaGupta-Bicycle merged commit 02608ab into ApptuitAI:master May 3, 2023
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.

2 participants