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

Cherry-pick #15686 to 7.x: Remove datasource option from SQL module and add tests #15697

Merged
merged 1 commit into from
Jan 21, 2020

Conversation

jsoriano
Copy link
Member

Cherry-pick of PR #15686 to 7.x branch. Original message:

What does this PR do?

  • Remove datasource option from SQL module. This option was intended to set the DSN of a database connection, and we were ignoring the hosts setting. In other SQL modules we are using the values in host as DSNs, do here the same for consistency. Host is redacted when we cannot parse it as it can contain passwords.
  • StandardizeEvent is exposed in mbtest.Fetcher interface so we can more easily check contents of events.
  • Add integration tests of the module with MySQL and PostgreSQL.
  • Add real data.json with data from MySQL and PostgreSQL.

Why is it important?

  • To have consistency between this new SQL module and other engine-specific SQL modules.
  • To have automatic tests.

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works

Related issues

Remove datasource option from SQL module. This option was
intended to set the DSN of a database connection, and we were
ignoring the hosts setting. In other SQL modules we are using
the values in hosts as DSNs, do here the same for consistency.
Host is redacted when we cannot parse it as it can contain passwords.

StandardizeEvent is exposed in mbtest.Fetcher interface so we can
more easily check contents of events in tests.

Add integration tests of the module with MySQL and PostgreSQL.

Add real data.json with data from MySQL and PostgreSQL.

(cherry picked from commit 8c71abc)
Copy link
Contributor

@kaiyan-sheng kaiyan-sheng left a comment

Choose a reason for hiding this comment

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

There is no changelog in the original PR, is that ok?

@jsoriano
Copy link
Member Author

There is no changelog in the original PR, is that ok?

Yep, this is a change in an unreleased feature.

@jsoriano jsoriano merged commit 10899ce into elastic:7.x Jan 21, 2020
@jsoriano jsoriano deleted the backport_15686_7.x branch January 21, 2020 15:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants