Opened 8 months ago

Closed 8 months ago

#5632 closed task (complete)

HA: use upper case prefix for server classes, e.g. HA_server1

Reported by: marcin Owned by: marcin
Priority: medium Milestone: Kea1.4-final
Component: high-availability 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: 0
Total Hours: 0 Internal?: no

Description

Builtin classes seem to be using upper case prefixes, e.g. VENDOR_. Our HA solution should also go down that path and use HA_server1 instead of ha_server1.

Subtickets

Change History (8)

comment:1 Changed 8 months ago by marcin

  • Milestone changed from Kea-proposed to Kea1.4-final

We have agreed with Tomek a week ago that this ticket should go into 1.4.0 final. This is incompatible change that we don't want to introduce after major release.

comment:2 follow-up: Changed 8 months ago by fdupont

I confirm there is an implicit convention to start builtin classes with uppercases (whole name or up to a separator).
Note there is a function with the list of builtin-class prefixes so please add HA_ in it so when an expression will use a HA class membership it won't raise an error because the class was not already defined (clearly something you should want)...

comment:3 in reply to: ↑ 2 Changed 8 months ago by marcin

Replying to fdupont:

I confirm there is an implicit convention to start builtin classes with uppercases (whole name or up to a separator).
Note there is a function with the list of builtin-class prefixes so please add HA_ in it so when an expression will use a HA class membership it won't raise an error because the class was not already defined (clearly something you should want)...

what function? Where is it?

comment:4 Changed 8 months ago by fdupont

Function isClientClassBuiltIn() prefix list builtinPrefixes in client_class_def.cc file.

comment:5 Changed 8 months ago by marcin

  • Owner set to UnAssigned
  • Status changed from new to reviewing

I replaced all occurrences of "ha_" with "HA_" in the HA hook library. I also updated the User's Guide as needed. The "HA_" is now designating a built-in class, so there is no need to declare the HA server classes in the configuration anymore.

Proposed ChangeLogs:

Main repo:

14XX.	[func]*		marcin
	The client classes used by the High Availability hook library
	use upper case "HA_" prefix and they are now built-in classes.
	This means that those classes do not need to be	declared in the
	server configuration.
	(Trac #5632, git cafe)
XX.	[func]*		marcin
	The HA hook library uses "HA_" (upper case instead of lower
	case) prefix for associating load balancing servers with client
	classes. This adheres to the convention of naming built-in
	classes in Kea.
	(Trac #5632, git cafe)

comment:6 Changed 8 months ago by fdupont

  • Owner changed from UnAssigned to fdupont

comment:7 Changed 8 months ago by fdupont

  • Owner changed from fdupont to marcin

In classify.xml: the client classes assigned -> (the) assigned client classes

In theory the HA_ prefix should be the second in the list (AFTER_ is a provision and EXTERNAL_ is external (in fact for hook writers))...

None of these is critical: the real issue is Kea trac5632 conflicts with trac5549a (known/unknown) so the second merge will require extra care...

Looking at the premium changes: reading it I can't see something more than ha_ -> HA_ so IMHO it is ready.

comment:8 Changed 8 months ago by marcin

  • Resolution set to complete
  • Status changed from reviewing to closed

Applied the changes according to the review and merged with commits 2d590bfd7d1b0eca377eb99eef83a3083a1d7399 and 6bfbf0a163485fb2ecf7c2bf4d6e49d8e8a82373.

Note: See TracTickets for help on using tickets.