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

Use post to query hosts #193

Conversation

ckyrouac
Copy link
Contributor

My linter cleaned up a bunch of stuff automatically. If you need me to rip those changes out to make it easier to review, let me know.

@Glutexo
Copy link
Collaborator

Glutexo commented Apr 15, 2019

This looks reviewable even with the style changes, even though they are not relevant to the PR subject. The problems with those are though:

  • it will heavily break other pending PRs, they will have to be manually rebased
  • we should agree on some style or linter+configuration before pushing any complex style changes

See #189. @jhjaggars suggested using Black.

@dehort
Copy link
Collaborator

dehort commented Apr 23, 2019

@fijshion we've made a number of changes to the code lately. I've merged master into the use_post_to_query_hosts branch. Can you resolve these conflicts and remove the formatting changes? Sorry for the trouble! I appreciate the help.

@Glutexo
Copy link
Collaborator

Glutexo commented Apr 23, 2019

I’ll review this after the rebase.

@dehort
Copy link
Collaborator

dehort commented Apr 23, 2019

@fijshion Can you modify the queries to by default return the host data without the "facts" and "system_profile"? Then add some flags/options to allow returning the "facts" and "system_profile"? Let me know what you think.

@ckyrouac
Copy link
Contributor Author

The rebase was really nasty so I just created a new branch. See #233. Closing this PR.

@ckyrouac ckyrouac closed this Apr 24, 2019
@Glutexo
Copy link
Collaborator

Glutexo commented Apr 24, 2019

Thanks a lot! Will take a look.

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