fix(parser/xpath): Allow new configuration flag to be set by Hipska · Pull Request #11183 · influxdata/telegraf

Hipska · 2022-05-24T15:22:19Z

Required for all PRs:

Updated associated README.md.
Wrote appropriate unit tests.
Pull request title or commits are in conventional commit format

resolves unusable config flag introduced in #11102

…path_error

powersj

This looks good - if you could rebase on master so tests can pass, we can land this!

Hipska · 2022-05-24T20:34:04Z

Yeah I merged master in it, do I need to do it again?

powersj · 2022-05-24T20:46:30Z

I re-kicked tests and on this run I see:

Auto-merging plugins/parsers/xpath/parser_test.go
CONFLICT (content): Merge conflict in plugins/parsers/xpath/parser_test.go
error: could not apply 67745efcf... chore(parsers.xpath): cleanup test arguments order
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 67745efcf... chore(parsers.xpath): cleanup test arguments order

Exited with code exit status 1

Hipska · 2022-05-25T04:30:05Z

Strange, GitHub itself doesn’t complain and doesn’t see conflicts

…path_error

Hipska · 2022-05-25T07:38:03Z

I merged master again..

powersj · 2022-05-25T18:56:54Z

I merged master again..

❯ cd /tmp
/tmp 
❯ git clone https://github.com/Super-Visions/telegraf -b feature/parsers/xpath_error
Cloning into 'telegraf'...
remote: Enumerating objects: 64568, done.
remote: Counting objects: 100% (252/252), done.
remote: Compressing objects: 100% (151/151), done.
remote: Total 64568 (delta 121), reused 199 (delta 100), pack-reused 64316
Receiving objects: 100% (64568/64568), 44.97 MiB | 10.64 MiB/s, done.
Resolving deltas: 100% (38365/38365), done.
/tmp took 5s 
❯ cd telegraf/
telegraf on  feature/parsers/xpath_error via 🐹 v1.18.2 
❯ git remote add upstream https://github.com/influxdata/telegraf
telegraf on  feature/parsers/xpath_error via 🐹 v1.18.2 
❯ git fetch upstream 
remote: Enumerating objects: 2356, done.
remote: Counting objects: 100% (1423/1423), done.
remote: Compressing objects: 100% (296/296), done.
remote: Total 2356 (delta 1135), reused 1385 (delta 1126), pack-reused 933
Receiving objects: 100% (2356/2356), 1.30 MiB | 9.54 MiB/s, done.
Resolving deltas: 100% (1427/1427), completed with 617 local objects.
From https://github.com/influxdata/telegraf
 * [new branch]          bubbletea                                     -> upstream/bubbletea
 * [new branch]          changeloggen                                  -> upstream/changeloggen
 * [new branch]          codeqlaction                                  -> upstream/codeqlaction
 * [new branch]          config-api-2                                  -> upstream/config-api-2
 * [new branch]          configurationupdate                           -> upstream/configurationupdate
 * [new branch]          dependabot/go_modules/github.com/sensu/sensu-go/api/core/v2-2.14.0 -> upstream/dependabot/go_modules/github.com/sensu/sensu-go/api/core/v2-2.14.0
 * [new branch]          dependabot/go_modules/github.com/tidwall/gjson-1.14.1 -> upstream/dependabot/go_modules/github.com/tidwall/gjson-1.14.1
 * [new branch]          dependabot/go_modules/go.opentelemetry.io/otel/metric-0.30.0 -> upstream/dependabot/go_modules/go.opentelemetry.io/otel/metric-0.30.0
 * [new branch]          dependabot/go_modules/k8s.io/api-0.24.0       -> upstream/dependabot/go_modules/k8s.io/api-0.24.0
 * [new branch]          dependabot/go_modules/k8s.io/client-go-0.24.0 -> upstream/dependabot/go_modules/k8s.io/client-go-0.24.0
 * [new branch]          deprecated-plugins                            -> upstream/deprecated-plugins
 * [new branch]          feat/nightly-docker                           -> upstream/feat/nightly-docker
 * [new branch]          fixcsvparser                                  -> upstream/fixcsvparser
 * [new branch]          fixstatewithconfig                            -> upstream/fixstatewithconfig
 * [new branch]          gm/docker-input-thinpool                      -> upstream/gm/docker-input-thinpool
 * [new branch]          gm/smart-input-extra-attrs                    -> upstream/gm/smart-input-extra-attrs
 * [new branch]          gm/stateful-parsers                           -> upstream/gm/stateful-parsers
 * [new branch]          isolateplugins                                -> upstream/isolateplugins
 * [new branch]          kafka-retry-connect                           -> upstream/kafka-retry-connect
 * [new branch]          master                                        -> upstream/master
 * [new branch]          mqtt-topic-parsing-length                     -> upstream/mqtt-topic-parsing-length
 * [new branch]          multipleoslint                                -> upstream/multipleoslint
 * [new branch]          parallelism                                   -> upstream/parallelism
 * [new branch]          patch-1                                       -> upstream/patch-1
 * [new branch]          prom_metric_format_doc                        -> upstream/prom_metric_format_doc
 * [new branch]          rabbitmq-listener                             -> upstream/rabbitmq-listener
 * [new branch]          readme-linter-agg-proc                        -> upstream/readme-linter-agg-proc
 * [new branch]          readme-linter-input-a-d                       -> upstream/readme-linter-input-a-d
 * [new branch]          release-1.0                                   -> upstream/release-1.0
 * [new branch]          release-1.0.1                                 -> upstream/release-1.0.1
 * [new branch]          release-1.1.0                                 -> upstream/release-1.1.0
 * [new branch]          release-1.10                                  -> upstream/release-1.10
 * [new branch]          release-1.11                                  -> upstream/release-1.11
 * [new branch]          release-1.12                                  -> upstream/release-1.12
 * [new branch]          release-1.13                                  -> upstream/release-1.13
 * [new branch]          release-1.14                                  -> upstream/release-1.14
 * [new branch]          release-1.15                                  -> upstream/release-1.15
 * [new branch]          release-1.16                                  -> upstream/release-1.16
 * [new branch]          release-1.17                                  -> upstream/release-1.17
 * [new branch]          release-1.18                                  -> upstream/release-1.18
 * [new branch]          release-1.19                                  -> upstream/release-1.19
 * [new branch]          release-1.2                                   -> upstream/release-1.2
 * [new branch]          release-1.20                                  -> upstream/release-1.20
 * [new branch]          release-1.21                                  -> upstream/release-1.21
 * [new branch]          release-1.22                                  -> upstream/release-1.22
 * [new branch]          release-1.3                                   -> upstream/release-1.3
 * [new branch]          release-1.4                                   -> upstream/release-1.4
 * [new branch]          release-1.5                                   -> upstream/release-1.5
 * [new branch]          release-1.6                                   -> upstream/release-1.6
 * [new branch]          release-1.7                                   -> upstream/release-1.7
 * [new branch]          release-1.8                                   -> upstream/release-1.8
 * [new branch]          release-1.9                                   -> upstream/release-1.9
 * [new branch]          release-update-website                        -> upstream/release-update-website
 * [new branch]          sampleconfigui                                -> upstream/sampleconfigui
 * [new branch]          snmnp-trap-upd-socket                         -> upstream/snmnp-trap-upd-socket
 * [new branch]          snmp_trap-integration                         -> upstream/snmp_trap-integration
 * [new branch]          speedtest-download-upload                     -> upstream/speedtest-download-upload
 * [new branch]          telegraf-lite-builder                         -> upstream/telegraf-lite-builder
 * [new branch]          testing                                       -> upstream/testing
 * [new branch]          tigerbotupdateconfig                          -> upstream/tigerbotupdateconfig
 * [new branch]          update-docker-mem-usage                       -> upstream/update-docker-mem-usage
 * [new branch]          vh-wpc-remote-computers-support               -> upstream/vh-wpc-remote-computers-support
 * [new branch]          windows_service                               -> upstream/windows_service
telegraf on  feature/parsers/xpath_error via 🐹 v1.18.2 
❯ git status
On branch feature/parsers/xpath_error
Your branch is up to date with 'origin/feature/parsers/xpath_error'.

nothing to commit, working tree clean
telegraf on  feature/parsers/xpath_error via 🐹 v1.18.2 
❯ git rebase upstream/master 
Auto-merging plugins/parsers/xpath/parser_test.go
CONFLICT (content): Merge conflict in plugins/parsers/xpath/parser_test.go
error: could not apply 67745efcf... chore(parsers.xpath): cleanup test arguments order
hint: Resolve all conflicts manually, mark them as resolved with
hint: "git add/rm <conflicted_files>", then run "git rebase --continue".
hint: You can instead skip this commit: run "git rebase --skip".
hint: To abort and get back to the state before "git rebase", run "git rebase --abort".
Could not apply 67745efcf... chore(parsers.xpath): cleanup test arguments order
telegraf on  HEAD (7656366) (REBASING 2/7) [=] via 🐹 v1.18.2 took 3s 
❯ git status
interactive rebase in progress; onto f4eefeadc
Last commands done (2 commands done):
   pick 160069e8a fix(parsers.xpath): Do not error if no nodes found for current config
   pick 67745efcf chore(parsers.xpath): cleanup test arguments order
Next commands to do (5 remaining commands):
   pick e65bae6b7 docs(parsers.xpath): Add new section also to second example config
   pick e6dc9be6c docs(parsers.xpath): Fix linter error
  (use "git rebase --edit-todo" to view and edit)
You are currently rebasing branch 'feature/parsers/xpath_error' on 'f4eefeadc'.
  (fix conflicts and then run "git rebase --continue")
  (use "git rebase --skip" to skip this patch)
  (use "git rebase --abort" to check out the original branch)

Unmerged paths:
  (use "git restore --staged <file>..." to unstage)
  (use "git add <file>..." to mark resolution)
	both modified:   plugins/parsers/xpath/parser_test.go

no changes added to commit (use "git add" and/or "git commit -a")
telegraf on  HEAD (7656366) (REBASING 2/7) [=] via 🐹 v1.18.2 
❯ git diff
diff --cc plugins/parsers/xpath/parser_test.go
index ee4c1444f,3a08f2b7c..000000000
--- a/plugins/parsers/xpath/parser_test.go
+++ b/plugins/parsers/xpath/parser_test.go
@@@ -1155,73 -1155,6 +1155,76 @@@ func TestEmptySelection(t *testing.T) 
                        _, err := parser.Parse([]byte(tt.input))
                        require.Error(t, err)
                        require.Equal(t, "cannot parse with empty selection node", err.Error())
++<<<<<<< HEAD
 +              })
 +      }
 +}
 +
 +func TestEmptySelectionAllowed(t *testing.T) {
 +      var tests = []struct {
 +              name    string
 +              input   string
 +              configs []Config
 +      }{
 +              {
 +                      name:  "empty path",
 +                      input: multipleNodesXML,
 +                      configs: []Config{
 +                              {
 +                                      Selection: "/Device/NonExisting",
 +                                      Fields:    map[string]string{"value": "number(Value)"},
 +                                      FieldsInt: map[string]string{"mode": "Value/@mode"},
 +                                      Tags:      map[string]string{},
 +                              },
 +                      },
 +              },
 +              {
 +                      name:  "empty pattern",
 +                      input: multipleNodesXML,
 +                      configs: []Config{
 +                              {
 +                                      Selection: "//NonExisting",
 +                                      Fields:    map[string]string{"value": "number(Value)"},
 +                                      FieldsInt: map[string]string{"mode": "Value/@mode"},
 +                                      Tags:      map[string]string{},
 +                              },
 +                      },
 +              },
 +              {
 +                      name:  "empty axis",
 +                      input: multipleNodesXML,
 +                      configs: []Config{
 +                              {
 +                                      Selection: "/Device/child::NonExisting",
 +                                      Fields:    map[string]string{"value": "number(Value)"},
 +                                      FieldsInt: map[string]string{"mode": "Value/@mode"},
 +                                      Tags:      map[string]string{},
 +                              },
 +                      },
 +              },
 +              {
 +                      name:  "empty predicate",
 +                      input: multipleNodesXML,
 +                      configs: []Config{
 +                              {
 +                                      Selection: "/Device[@NonExisting=true]",
 +                                      Fields:    map[string]string{"value": "number(Value)"},
 +                                      FieldsInt: map[string]string{"mode": "Value/@mode"},
 +                                      Tags:      map[string]string{},

powersj · 2022-05-25T19:03:33Z

I merged master again..

I think I would make sure your master has fetched the latest because it seems to be missing some changes.

Hipska · 2022-05-26T04:08:30Z

As you can see in the commit, it was with the latest master commit 🤷🏻‍♂️

powersj · 2022-05-26T13:12:01Z

Yeah I'm not sure why it says that, but tests say otherwise. If you go to your branch in github the UI will say:

This branch is 9 commits ahead, 11 commits behind influxdata:master.

Hipska · 2022-05-29T10:41:50Z

Yeah, that is all logical. It will keep adding those numbers for each commit done to master.. So what can we do?

IMHO there is a problem with the commands run in the tests.. You can also see in the diff here, the changes I have done don't do anything that would conflict in the files the tests are saying there are conflicts..

Hipska · 2022-05-30T15:00:28Z

Will close this one as tests keep failing and will also be included by #11218.

Hipska added 8 commits May 16, 2022 17:10

fix(parsers.xpath): Do not error if no nodes found for current config

160069e

chore(parsers.xpath): cleanup test arguments order

67745ef

docs(parsers.xpath): Add new section also to second example config

e65bae6

docs(parsers.xpath): Fix linter error

e6dc9be

feat(parsers/xpath): Rewrite ParseLine to support multiple configs

14ca651

fix(parser/xpath): Allow new configuration flag to be set

6aa7e57

Merge remote-tracking branch 'upstream/master' into feature/parsers/x…

52d8a38

…path_error

doc(parsers/xpath): Remove duplicates

f2b8c7c

telegraf-tiger bot added the fix pr to fix corresponding bug label May 24, 2022

Hipska mentioned this pull request May 24, 2022

fix(parsers/xpath): Do not error if no nodes found for current config #11102

Merged

3 tasks

Hipska requested a review from powersj May 24, 2022 15:23

powersj reviewed May 24, 2022

View reviewed changes

Merge remote-tracking branch 'upstream/master' into feature/parsers/x…

ac95124

…path_error

Hipska added area/xml plugin/parser 1. Request for new parser plugins 2. Issues/PRs that are related to parser plugins labels May 25, 2022

Hipska mentioned this pull request May 30, 2022

feat: Migrate xpath parser to new style #11218

Merged

3 tasks

Hipska closed this May 30, 2022

Hipska deleted the feature/parsers/xpath_error branch May 30, 2022 15:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(parser/xpath): Allow new configuration flag to be set#11183

fix(parser/xpath): Allow new configuration flag to be set#11183
Hipska wants to merge 9 commits intoinfluxdata:masterfrom
Super-Visions:feature/parsers/xpath_error

Hipska commented May 24, 2022

Uh oh!

powersj left a comment

Uh oh!

Hipska commented May 24, 2022 •

edited

Loading

Uh oh!

powersj commented May 24, 2022

Uh oh!

Hipska commented May 25, 2022

Uh oh!

Hipska commented May 25, 2022

Uh oh!

powersj commented May 25, 2022

Uh oh!

powersj commented May 25, 2022

Uh oh!

Hipska commented May 26, 2022

Uh oh!

powersj commented May 26, 2022

Uh oh!

Hipska commented May 29, 2022

Uh oh!

Hipska commented May 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Hipska commented May 24, 2022

Required for all PRs:

Uh oh!

powersj left a comment

Choose a reason for hiding this comment

Uh oh!

Hipska commented May 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

powersj commented May 24, 2022

Uh oh!

Hipska commented May 25, 2022

Uh oh!

Hipska commented May 25, 2022

Uh oh!

powersj commented May 25, 2022

Uh oh!

powersj commented May 25, 2022

Uh oh!

Hipska commented May 26, 2022

Uh oh!

powersj commented May 26, 2022

Uh oh!

Hipska commented May 29, 2022

Uh oh!

Hipska commented May 30, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Hipska commented May 24, 2022 •

edited

Loading