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

added solr startup check. Refactored to make props available #1656

Merged
merged 2 commits into from
Jul 6, 2023

Conversation

artntek
Copy link
Contributor

@artntek artntek commented Jul 4, 2023

see #1631

  • added solr startup check.
  • Also refactored to make properties available for this solr check (and for future checks).
  • Added tests

@artntek artntek requested a review from taojing2002 July 4, 2023 00:56

// can read?
defaultProperties.load(Files.newBufferedReader(defaultPropsFilePath));
// can write? Use isWriteable() so we don't mess up props file formatting for Jing :-)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks. But the metacat.properties file is still changed :). I believe it happens when we write some properties (such as status) into the file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agreed - I just thought I would be better not to have it happen in yet another place :-)


Properties siteProperties = new Properties();
runtimeProperties = new Properties(defaultProperties);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason why we need the defaultProperties to initialize the runtimeProperties? If we just use the default constructor to initialize the runtimeProperties object, we can only has one parameter in the method signature.

Copy link
Contributor

@taojing2002 taojing2002 left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you!

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