Skip to content

[service_discovery][jmx] fix yaml parsing#3139

Merged
sjenriquez merged 1 commit intomasterfrom
scott/sd-jmx-autoconf
Jan 24, 2017
Merged

[service_discovery][jmx] fix yaml parsing#3139
sjenriquez merged 1 commit intomasterfrom
scott/sd-jmx-autoconf

Conversation

@sjenriquez
Copy link
Contributor

What does this PR do?

YAML parsing for JMX-service discovery was broken after merging #2901

This small change fixes this bug.

@sjenriquez sjenriquez requested a review from truthbk January 20, 2017 19:31
@hkaj
Copy link
Member

hkaj commented Jan 23, 2017

That should work yeah, but tests are not happy somehow:

======================================================================
ERROR: Test JMX configs are read and converted to YAML
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7.9/lib/python2.7/site-packages/mock.py", line 1201, in patched
    return func(*args, **keywargs)
  File "/home/travis/build/DataDog/dd-agent/tests/core/test_service_discovery.py", line 593, in test_read_jmx_config_from_store
    jmx_configs = generate_jmx_configs(self.auto_conf_agentConfig, "jmxhost")
  File "/home/travis/build/DataDog/dd-agent/config.py", line 1144, in generate_jmx_configs
    _, (sd_init_config, sd_instances) = service_disco_check_config
ValueError: need more than 1 value to unpack
-------------------- >> begin captured logging << --------------------
config: INFO: Fetching service discovery check configurations.
config: DEBUG: Generating JMX config for: solr
config: ERROR: Unable to generate YAML config for solr
Traceback (most recent call last):
  File "/home/travis/build/DataDog/dd-agent/config.py", line 1150, in generate_jmx_configs
    yaml = config_to_yaml(check_config)
  File "/home/travis/build/DataDog/dd-agent/util.py", line 141, in config_to_yaml
    raise Exception('You need to have at least one instance defined in your config.')
Exception: You need to have at least one instance defined in your config.
config: DEBUG: Generating JMX config for: tomcat
--------------------- >> end captured logging << ---------------------```

@sjenriquez
Copy link
Contributor Author

@hkaj Tests are passing now after updating our template. Not entirely sure how they were passing after #2901 was merged though.

Copy link
Member

@hkaj hkaj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👌 squash the commits and it's good to go

@masci masci added this to the Triage milestone Jan 24, 2017
@sjenriquez sjenriquez force-pushed the scott/sd-jmx-autoconf branch from f7e69d4 to 78a9053 Compare January 24, 2017 19:01
Copy link
Member

@truthbk truthbk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@sjenriquez sjenriquez merged commit 4d273b6 into master Jan 24, 2017
@sjenriquez sjenriquez deleted the scott/sd-jmx-autoconf branch January 24, 2017 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants