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

Update mysql_db.py, adding encoding to enforce charset for import #61216

Closed
wants to merge 3 commits into from
Closed

Update mysql_db.py, adding encoding to enforce charset for import #61216

wants to merge 3 commits into from

Conversation

netmonk
Copy link
Contributor

@netmonk netmonk commented Aug 23, 2019

SUMMARY

encoding is used for creation of database, but not for import of sql scripts.
The import fails to import latin1 encoded sql script, cause it consider it's by default some utf8

This commit add --default-character-set=encoding when the import function is called with an encoding argument.
This way this option is added to the mysql command line, and import is successfull.

ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
  • New Module Pull Request
COMPONENT NAME

mysql_db

ADDITIONAL INFORMATION

`encoding` is used for creation of database, but not for import of sql scripts.
The import fails to import `latin1` encoded sql script, cause it consider it's by default some utf8

This commit add `--default-character-set=encoding` when the import function is called with an encoding argument. 
This way this option is added to the mysql command line, and import is successfull.
@ansibot ansibot added affects_2.7 This issue/PR affects Ansible v2.7 backport This PR does not target the devel branch. bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. database Database category module This issue/PR relates to a module. mysql needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community. labels Aug 23, 2019
@ansibot
Copy link
Contributor

ansibot commented Aug 23, 2019

The test ansible-test sanity --test pep8 [explain] failed with 1 error:

lib/ansible/modules/database/mysql/mysql_db.py:182:161: E501 line too long (165 > 160 characters)

click here for bot help

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed community_review In order to be merged, this PR must follow the community review workflow. labels Aug 23, 2019
Fix the linter issue and add another control to test encoding var.
@ansibot ansibot added community_review In order to be merged, this PR must follow the community review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Aug 25, 2019
Fix again lint error ! :)
@abadger
Copy link
Contributor

abadger commented Aug 30, 2019

Hi, two things:

  • Changes should be submitted to the devel branch and then backported after merging. That way we don't reintroduce a bug when we release new versions
  • 2.7 is only open for critical bugfixes right now. This is probably not going to meet the definition of a critical bugfix (it doesn't affect the core engine. It doesn't prevent any use of the mysql_db module. etc). However, 2.8 and 2.9 (in development) are still accepting non-critical bugfixes so this can be backported to those once it is in the devel branch.

Could you please reopen this against devel?

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Aug 30, 2019
@netmonk
Copy link
Contributor Author

netmonk commented Aug 30, 2019

Hello @abadger i understand your point.
I already have another PR opened for upstream #61215 in the same time than this one.
I wanted to backport in 2.7 as far as i use RHEL7 and they only distribute ansible 2.7 on LTS package.

If you can have a look at the PR on devel branch i submitted, would be nice too :)

@netmonk netmonk closed this Aug 30, 2019
@ansible ansible locked and limited conversation to collaborators Oct 1, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.7 This issue/PR affects Ansible v2.7 backport This PR does not target the devel branch. bug This issue/PR relates to a bug. community_review In order to be merged, this PR must follow the community review workflow. database Database category module This issue/PR relates to a module. mysql new_contributor This PR is the first contribution by a new community member. support:community This issue/PR relates to code supported by the Ansible community.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants