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

Remove unknown options to the mysql2 initalizer #517

Open
FanGoH opened this issue Oct 5, 2023 · 3 comments
Open

Remove unknown options to the mysql2 initalizer #517

FanGoH opened this issue Oct 5, 2023 · 3 comments
Labels

Comments

@FanGoH
Copy link

FanGoH commented Oct 5, 2023

Suggestion

Is there any drawback to stripping connection options from the DataSource to the Mysql2 underlying connector?, I would ideally like to remove the emission of this warning

 if (validOptions[key] !== 1) {
        // REVIEW: Should this be emitted somehow?
        // eslint-disable-next-line no-console
        console.error(
          `Ignoring invalid configuration option passed to Connection: ${key}. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection`
        );
      }

If there isn't anything against it, I would love to contribute with a pull request, the only drawback I can think of is that the validOptions aren't exported from the library.

(Sorry for not following the template, I didn't felt that the formats properly addressed what I wanted to communicate)

@FanGoH FanGoH added the feature label Oct 5, 2023
@KunalBurangi
Copy link

I am having a similar issue after updating to 7.0.6

Ignoring invalid configuration option passed to Connection: name. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection

Ignoring invalid configuration option passed to Connection: connector. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection

Ignoring invalid configuration option passed to Connection: url. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection

Ignoring invalid configuration option passed to Connection: disableMigration. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection

Ignoring invalid configuration option passed to Connection: lazyConnect. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection

Ignoring invalid configuration option passed to Connection: collation. This is currently a warning, but in future versions of MySQL2, an error will be thrown if you pass an invalid configuration option to a Connection

Are these options not supported?
if not what are the alternatives for these options?
I want to use lazyConnect and disableMigration

What is the solution to avoid this ?

The documentation still shows that we can use all these parameters

@dhurtrci
Copy link

dhurtrci commented Mar 8, 2024

I have the same issue.
The manipulations in generateOptions in mysql.js add in 'collation' property in options object that then gets flagged as error in the console due to code highlighted by @FanGoH .

Additionally, for Loopback juggler.DataSource options, connector and name properties are needed in the options object, which also end up getting passed through to the code highlighted by @FanGoH (via the code added in #46).

    for (const p in s) {
      if (p === 'database' && s.createDatabase) {
        continue;
      }
      if (options[p] === undefined) {
        options[p] = s[p];
      }
    }

Whilst it is desirable to pass through valid options (in my case it is how I pass ssl options), perhaps an answer is for mysql2 to export the list of valid options (probably have them in separate module in fact) and for the 'option passthrough' code above to only include valid options.... E.g. (assuming e.g. that validOptions is exported by mysql2.js in mysql2 module and imported in mysql.js).

  for (const p in s) {
      if (p === 'database' && s.createDatabase) {
        continue;
      }
      if (options[p] === undefined && validOptions[p] === 1) {
        options[p] = s[p];
      }
    }

Of course this would need the change (to export the valid options) to be made also in the mysql2 project, and we definitely do not want to duplicate the list of valid options in loopback-connector-mysql....

A disadvantage of the change is that it is possible that the mysql2 maintained list of valid options isn't up-to-date with the latest list of options accepted by MySQL/MariaDB - and in that case there is an argument to get rid of the logged error (flagged by @FanGoH ) entirely!

@dhurtrci
Copy link

dhurtrci commented Mar 8, 2024

I've also raised issue in mysql2 for this.
My preference is that the logging (and threatened future error throwing) is removed entirely....

sidorares/node-mysql2#2481

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants