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

Add support for enable_load_data_local_infile variable #109

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jpiron
Copy link
Contributor

@jpiron jpiron commented Sep 9, 2022

SUMMARY

Add support for setting enable_load_data_local_infile variable

ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

proxysql role

@markuman
Copy link
Member

markuman commented Sep 9, 2022

@jpiron thanks for your contribution.

The variable mysql-enable_load_data_local_infile was introduced with proxysql 2.3.0. So when you use this role with proxysql_version: 2.2.2, the installation will fail

failed: [localhost] (item={'key': 'enable_load_data_local_infile', 'value': {'variable': 'enable_load_data_local_infile', 'variable_value': False}}) => {"ansible_loop_var": "item", "changed": false, "item": {"key": "enable_load_data_local_infile", "value": {"variable": "enable_load_data_local_infile", "variable_value": false}}, "msg": "The variable \"mysql-enable_load_data_local_infile\" was not found"}

unfortunately, we have no compliance how long we support proxysql versions < 2.3.0.
Can you please cover this like you did the last time?

@jpiron
Copy link
Contributor Author

jpiron commented Sep 10, 2022

Sure I'll add this on monday

@jpiron
Copy link
Contributor Author

jpiron commented Sep 12, 2022

It's updated

@markuman
Copy link
Member

@jpiron I still got an error when trying to install 2.2.2 e.g.

---
- hosts: localhost
  become: yes

  vars:
    proxysql_version: 2.2.2

  roles:
    - proxysql

results in

RUNNING HANDLER [proxysql : proxysql | handler | manage mysql config] *****
...
failed: [localhost] (item={'key': 'enable_load_data_local_infile', 'value': {'variable': 'enable_load_data_local_infile', 'variable_value': False}}) => {"ansible_loop_var": "item", "changed": false, "item": {"key": "enable_load_data_local_infile", "value": {"variable": "enable_load_data_local_infile", "variable_value": false}}, "msg": "The variable \"mysql-enable_load_data_local_infile\" was not found"}
....

@markuman markuman mentioned this pull request Sep 12, 2022
@jpiron
Copy link
Contributor Author

jpiron commented Sep 14, 2022

I might be wrong but my understanding of the issue is that the default /etc/proxysql.cnf that ships with the 2.2.2 proxysql version doesn't contain the enable_load_data_local_infile` variable. Therefore the variable isn't loaded into proxysql runtime.

The manage mysql config handler calls the proxysql_global_variables that only performs updates on already proxysql-runtime-loaded variables.

As a result, it fails here: https://github.com/ansible-collections/community.proxysql/blob/main/plugins/modules/proxysql_global_variables.py#L232

Shouldn't the module perform an INSERT if the get_config returns False ?

@markuman
Copy link
Member

@jpiron sorry for the late reply, I was out of office.

Shouldn't the module perform an INSERT if the get_config returns False ?

That might work I think. Are we sure that there are not side-effects when inserting new global variables to proxysql?

@jpiron
Copy link
Contributor Author

jpiron commented Oct 11, 2022

I suppose, I need to check

@markuman
Copy link
Member

That might work I think. Are we sure that there are not side-effects when inserting new global variables to proxysql?

I've inserted a random name variable. It looks like that it doesn't made any problems...

@jpiron
Copy link
Contributor Author

jpiron commented Jan 24, 2023

Sorry for the late answer, I didn't get the time to work on this lately.
I'll try to manage some time to implement this.

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.

None yet

2 participants