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

detangle: Move osquery shutdown logic outside of Initializer #6530

Merged
merged 6 commits into from
Jul 9, 2020

Conversation

theopolis
Copy link
Member

@theopolis theopolis commented Jun 27, 2020

The goal is to have less inter-dependency within osquery components.
This is the first of several changes that take small steps towards a
simpler dependency graph.

Later we can revisit the directory structure to see if we can convey
what components are intended to be widely used and what components
are specialized.

See #5990.

@theopolis theopolis added merge with rebase refactor Related to osquery code refactoring labels Jun 27, 2020
The goal is to have less inter-dependency within osquery components.
This is the first of several changes that take small steps towards a
simpler dependency graph.

Later we can revisit the directory structure to see if we can convey
what components are intended to be widely used and what components
are specialized.
@theopolis
Copy link
Member Author

theopolis commented Jun 27, 2020

  • Builds passing for detangle: Move osquery shutdown logic outside of Initializer
  • Builds passing for detangle: Move tooltype setter and getters out of Initializer
  • Builds passing for detangle: Move start time getter and setter out of Config
  • Builds passing for detangle: Move platform setup and teardown out of Initializer.
  • Builds passing for detangle: Remove dependency on watcher from extensions.

@theopolis theopolis added the ready for review Pull requests that are ready to be reviewed by a maintainer label Jun 29, 2020
@theopolis theopolis added this to the 4.5.0 milestone Jul 4, 2020
Copy link
Member

@directionless directionless left a comment

Choose a reason for hiding this comment

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

I looked over this (up to 29ef8fd), but I don't feel super comfortable with this part of the code. From what I can tell, it looks cleaner. I don't know if anyone else will take a look, but I can thumb this if no one else steps up

* @param retcode the request return code for the process.
* @param system_log A log line to write to the system's log.
*/
void requestShutdown(int retcode, const std::string& system_log);
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it makes sense to name this forceShutdown instead of just having the extra system_log argument?

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe, but for this PR the thesis is detangling. We can refactor/rename things as separate as separate thesis later.

@theopolis theopolis merged commit 5c084ec into osquery:master Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge with rebase ready for review Pull requests that are ready to be reviewed by a maintainer refactor Related to osquery code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants