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

clickhouse_cfg_info: add the new module #84

Merged

Conversation

Andersson007
Copy link
Contributor

SUMMARY

clickhouse_cfg_info: add the new module

this is MVP

If it's OK, let's merge this one and then discuss future possibilities in a new issue

@Andersson007
Copy link
Contributor Author

@aleksvagachev ready for review

@Andersson007
Copy link
Contributor Author

ready for review

@aleksvagachev
Copy link
Collaborator

@Andersson007 Thanks for developing the new module!

  1. Are you planning to add xml in the future?
  2. Do we need to add a file existence check?
  3. This module must be executed on a clickhouse server. This is not necessary for other modules. They are executed on one host, and the connection and execution of sql is on the other. Should we stick to such a system or not?

@Andersson007
Copy link
Contributor Author

@Andersson007 Thanks for developing the new module!

  1. Are you planning to add xml in the future?
  2. Do we need to add a file existence check?
  3. This module must be executed on a clickhouse server. This is not necessary for other modules. They are executed on one host, and the connection and execution of sql is on the other. Should we stick to such a system or not?

@aleksvagachev thanks for asking:

  1. Yes, but would be happy to delegate it if you or anybody else who wants to become a co-author. I've already googled and it should be easy to implement with a couple of libs
  2. Maybe, but not in this PR. A bit of more graceful error handling, would probably make sense
  3. I believe no, it's OK

@aleksvagachev aleksvagachev merged commit 5e5df44 into ansible-collections:main Oct 29, 2024
21 checks passed
@Andersson007
Copy link
Contributor Author

@aleksvagachev thanks for reviewing!
would you like to add xml support and the file error handling feature yourself? I could do it if not, no problem. Please let me know

@aleksvagachev
Copy link
Collaborator

@Andersson007 If you have free time, please implement it. If not, I can do it.

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