Opened 3 years ago

Closed 3 years ago

#5061 closed enhancement (fixed)

Unable to configure port for mysql

Reported by: parisioa Owned by: fdupont
Priority: medium Milestone: Kea1.2
Component: database-all Version: git
Keywords: Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 3
Total Hours: 3 Internal?: no

Description

We use a non-standard port for our MySQL servers and the code base does not provide a configuration option for port.

Subtickets

Attachments (1)

mysql_connection.cc.diff (1.4 KB) - added by parisioa 3 years ago.
The patch we applied to add this configuration option.

Download all attachments as: .zip

Change History (15)

Changed 3 years ago by parisioa

The patch we applied to add this configuration option.

comment:1 Changed 3 years ago by hschempf

  • Milestone changed from Kea-proposed to Kea1.2

Per Kea meeting Nov 3, accept 1.2 as low

comment:2 Changed 3 years ago by fdupont

Do be addressed when migration will be finished.

comment:3 Changed 3 years ago by fdupont

  • Priority changed from low to medium

Bumped priority as it was requested by a second customer.

comment:4 Changed 3 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

It can be done as #5019 was merged... Taking it.

comment:5 Changed 3 years ago by fdupont

Done. Running Jenkins for the MySQL part and thinking about an unit test (how the host parameter is checked?).
Almost ready for review.

comment:6 Changed 3 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

Added parser unit tests and PostgreSQL support. BTW Cassandra uses some keywords which are currently unknown (and port): contact_points (an alias of host) and keyspace. If needed this ticket can be extended to fix the grammar for Cassandra.

Ready for review.

comment:7 Changed 3 years ago by fdupont

  • Owner changed from UnAssigned to fdupont
  • Status changed from reviewing to accepted

Extended to fix Cassandra too.

comment:8 Changed 3 years ago by tomek

  • Component changed from Unclassified to dhcpdb

comment:9 Changed 3 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

Should be good now. Ready for review for both MySQL (port) and Cassandra (specific keywords including port) updates.

comment:10 Changed 3 years ago by tomek

  • Owner changed from UnAssigned to tomek

comment:11 follow-up: Changed 3 years ago by tomek

  • Add Hours to Ticket changed from 0 to 3
  • Owner changed from tomek to fdupont
  • Total Hours changed from 0 to 3

Thanks for working on this problem. I have reviewed your changes
and they're good. There was still a bit more do here, so went
ahead and done some changes.

dhcpX_lexer.ll
The code added double lexer entries for both contact_points
and contact-points. I think it's not necessary. Since we always said
the Cassandra support is experimental, it's ok to clean it up.
So please remove the "contact_points" regexp and keep only the
one with a dash.

This applies to both v4 and v6. I have updated the code to
use only "contact-points", added example config files that use it
and updated unit-tests to check that such configs can be parsed
properly. Please pull and review.

Also, User's Guide must be updated. I have done that. Please pull
and review.

We definitely need a changelog for this. How about this one:

12XX.	[func]		parisioa,fdupont,tomek
	DHCPv4 and DHCPv6 parsers have updated to accept port
        parameter. The parameter for Cassandra is now called
        "contact-points" (was "contact_points" previously).
	(Trac #5061, git tbd)

We also need to update AUTHORS file with parisioa being created.
I'll ask how he would like to be credited. Back to you, Francis.


I did compile the code with every backend enabled (MySQL, PostgreSQL and Cassandra).
Sadly, I was not able to run unit-tests for cassandra backend, because of this
bug: https://issues.apache.org/jira/browse/CASSANDRA-11850. As the code changes
do not change the backend itself, just the configuration, I think it's ok.

comment:12 Changed 3 years ago by tomek

Email to parisioa sent. Will update this ticket once the response comes in.

comment:13 in reply to: ↑ 11 Changed 3 years ago by fdupont

Replying to tomek:

dhcpX_lexer.ll
The code added double lexer entries for both contact_points
and contact-points. I think it's not necessary. Since we always said
the Cassandra support is experimental, it's ok to clean it up.
So please remove the "contact_points" regexp and keep only the
one with a dash.

=> the first time I read this I thought you were a bit rude but
I bought your argument and it was something we will have to
cleanup one day so why not now?

Merged, closing.

comment:14 Changed 3 years ago by fdupont

  • Resolution set to fixed
  • Status changed from reviewing to closed
Note: See TracTickets for help on using tickets.