Adds support for a YAML formatted parent.config file. by jrushford · Pull Request #4857 · apache/trafficserver

jrushford · 2019-01-24T01:06:02Z

This PR adds YAML configuration support to the trafficserver parent.config file. By default, The current style line by line format is parsed and loaded. To use a YAML parent configuration file edit records.config and change "proxy.config.http.parent_proxy.file" so that the file name is "parent.yaml" instead of "parent.config".

jrushford · 2019-01-25T15:59:51Z

[approve ci]

SolidWallOfCode · 2019-01-30T16:37:05Z

Sorry for the delay, but there's a first pass.

SolidWallOfCode · 2019-03-04T00:19:51Z

-
-    if (!label) {
-      continue; // Already use.
+  if (!line_info->config_is_yaml) {


Is this really switching formats on a per line basis? That seems a bit excessive. The file should be old style, or it should be YAML, but never both. I don't think ControlMatcher should ever be exposed to YAML configuration.

@SolidWallOfCode It's not switching formats on a per line basis. For YAML, each node is parsed sequentially. I wanted to add YAML support making it optional, you only need to change the filename used from parent.config to parent.yaml to use YAML. It doesn't change the behavior of the current parent.config parser which is per line, that remains. For YAML, the entire file is loaded and you have a YAML sequence of maps basically for each parent entry. I have the ControlMatcher just reading enough of the YAML, each YAML Node in the sequence, so that it can allocate space by destination type, just as is currently done for the line by line config file. Once space has been allocated for each destination type, HostMatcher, IPMatcher, URLMatcher etc... will make a call to ParentSelection::Init() to parse the parent selection specific stuff which is then added to the appropriate matchers data structure.

ControlMatcher is parsing line-by-line now. With this PR, it will be able to use the YAML format as well.

@SolidWallOfCode I made changes here and removed the unnecessary config_is_yaml bool. I am passing a pointer to the root yaml node though on each matcher_line but, it's either all yaml or all old style line format. The two are not mixed.

SolidWallOfCode · 2019-03-04T00:41:34Z

+     * sequence when it is called.
+     *
+     */
+    const std::string content{file_buf};


you might want to look at ts::file::load, which reads the file contents directly in to a std::string, and avoid file_buf. You can pass the content string instead of file_buf if that's still needed.

So this PR just adds YAML support without taking out the current line by line configuration. I just used the current code for reading the file.

@SolidWallOfCode I went with YAML::Load(char *) here as the config file is already read by the current code.

SolidWallOfCode · 2019-03-04T03:13:53Z

  } while (0)

+// set's the filename with a recognized yaml suffix for testing a yaml config.
+#define REBUILD_YAML                                                                                                       \


Why #define instead of an inline function or lambda?

@SolidWallOfCode This is only used for unit tests, see also REBUILD for the current unit-tests. Do you think I should change this?

Yes. Test code should be held to the same coding standards as regular code.

@SolidWallOfCode There are 212 tests using these test macros, T, REBUILD, REBUILD_YAML, REINIT, ST, RE, and FP. Would it be acceptable to create an issue to circle back and refactor the unit-tests in a separate PR as changing these to inline functions will require changing all the existing tests? Personally, I think this should be done in a separate PR.

jrushford · 2019-03-05T19:48:32Z

[approve ci autest]

SolidWallOfCode · 2019-04-17T19:11:43Z

+  const char *val;
+  matcher_type type = MATCH_NONE;
+
+  YAML::Node root = *(p->root_node);


Allright. The matcher_line contains a root_node which is presumed to be a list of objects, each of which has keys that correspond to the keys in tags. All the instances of p passed in point to the same root node? Why can't the matcher_line just point directly at the object?

bryancall · 2019-07-16T16:40:50Z

@jrushford Looks like the build is failing on this.

jrushford added Configuration WIP labels Jan 24, 2019

jrushford self-assigned this Jan 24, 2019

jrushford requested review from SolidWallOfCode, randall and vmamidi January 24, 2019 01:06

jrushford force-pushed the parent_config_yml branch 3 times, most recently from b1ab8a3 to 3b78b6d Compare January 24, 2019 23:04

jrushford force-pushed the parent_config_yml branch 2 times, most recently from 2cc60ef to 0ac1129 Compare January 25, 2019 20:47

ywkaras reviewed Jan 29, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.cc Outdated

ywkaras reviewed Jan 29, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.cc Outdated

ywkaras reviewed Jan 29, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.cc Outdated

ywkaras reviewed Jan 29, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.cc Outdated

ywkaras reviewed Jan 29, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.cc Outdated

ywkaras reviewed Jan 29, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.h Outdated

SolidWallOfCode reviewed Jan 30, 2019

View reviewed changes

Comment thread proxy/ParentSelection.cc Outdated

SolidWallOfCode reviewed Jan 30, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.cc Outdated

SolidWallOfCode reviewed Jan 30, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.h Outdated

SolidWallOfCode reviewed Jan 30, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.h Outdated

SolidWallOfCode reviewed Jan 30, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.h Outdated

SolidWallOfCode reviewed Jan 30, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.h Outdated

SolidWallOfCode reviewed Jan 30, 2019

View reviewed changes

Comment thread proxy/ParentYamlConfig.cc Outdated

jrushford force-pushed the parent_config_yml branch 4 times, most recently from 686a2da to cd073f5 Compare February 6, 2019 17:03

jrushford force-pushed the parent_config_yml branch 3 times, most recently from 9f1ea4f to e5cc7d6 Compare March 1, 2019 21:12

jrushford added Parent Proxy and removed WIP labels Mar 1, 2019

SolidWallOfCode reviewed Mar 4, 2019

View reviewed changes

jrushford force-pushed the parent_config_yml branch from d62e6da to ac98738 Compare March 6, 2019 17:15

jrushford changed the title ~~Update that adds a YAML format and parsing to the existing parent.config~~ Adds support for a YAML formatted parent.config file. Mar 6, 2019

jrushford force-pushed the parent_config_yml branch from ac98738 to 401ec25 Compare March 6, 2019 18:06

jrushford force-pushed the parent_config_yml branch from 401ec25 to c1abd68 Compare April 6, 2019 03:52

SolidWallOfCode reviewed Apr 17, 2019

View reviewed changes

Comment thread proxy/ControlBase.cc Outdated

SolidWallOfCode reviewed Apr 17, 2019

View reviewed changes

Comment thread configs/parent.schema.json Outdated

SolidWallOfCode reviewed Apr 17, 2019

View reviewed changes

Comment thread configs/parent.schema.json Outdated

SolidWallOfCode reviewed Apr 17, 2019

View reviewed changes

Comment thread configs/parent.schema.json Outdated

SolidWallOfCode reviewed Apr 17, 2019

View reviewed changes

Comment thread src/tscore/MatcherUtils.cc Outdated

jrushford force-pushed the parent_config_yml branch 3 times, most recently from ac4b5ef to c11dcd7 Compare May 20, 2019 21:13

apache deleted a comment from jrushford May 23, 2019

randall reviewed May 24, 2019

View reviewed changes

Comment thread configs/parent.schema.json Outdated

Comment thread configs/parent.schema.json Outdated

Comment thread configs/parent.schema.json Outdated

Adds YAML format and parsing for the parent.config file

7aebaf0

jrushford force-pushed the parent_config_yml branch from 3e60782 to 7aebaf0 Compare May 28, 2019 19:01

fix the parent schema per review comments.

2190189

zwoop mentioned this pull request Jun 13, 2019

☂ Action items prior to July 9.x branch #5457

Closed

18 tasks

apache deleted a comment from bryancall Jul 17, 2019

jrushford closed this Jul 18, 2019

zwoop removed this from the 9.0.0 milestone Aug 27, 2019

Conversation

jrushford commented Jan 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jrushford commented Jan 25, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SolidWallOfCode commented Jan 30, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrushford Mar 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrushford Mar 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrushford Mar 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jrushford commented Mar 5, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bryancall commented Jul 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

jrushford commented Jan 24, 2019 •

edited

Loading

jrushford Mar 4, 2019 •

edited

Loading

jrushford Mar 5, 2019 •

edited

Loading

jrushford Mar 6, 2019 •

edited

Loading