Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Yaml instead of JSON #3426

Closed
wants to merge 1 commit into from
Closed

Use Yaml instead of JSON #3426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .circleci/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ experimental:
templates:
job_template: &job_template
docker:
- image: datadog/datadog-agent-runner-circle:six
- image: datadog/datadog-agent-runner-circle:six-pip3
environment:
USE_SYSTEM_LIBS: "1"
working_directory: /go/src/github.com/DataDog/datadog-agent
Expand Down
2 changes: 1 addition & 1 deletion .circleci/images/runner/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ RUN set -ex \
gnupg ca-certificates \
gcc g++ make git ssh curl pkg-config file \
python-dev python-setuptools python-pip \
python3.7-dev python3-distutils \
python3.7-dev python3-distutils python3-pip python3-yaml \
arbll marked this conversation as resolved.
Show resolved Hide resolved
libssl-dev libsnmp-base libsnmp-dev libpq-dev snmp-mibs-downloader libsystemd-dev

# Golang
Expand Down
2 changes: 1 addition & 1 deletion .gitlab-ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ before_script:
# run tests for deb-x64
run_tests_deb-x64:
stage: source_test
image: datadog/datadog-agent-runner-circle:six
image: datadog/datadog-agent-runner-circle:six-pip3
tags: [ "runner:main", "size:2xlarge" ]
before_script:
- pip install wheel
Expand Down
1 change: 1 addition & 0 deletions appveyor.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ install:
- set PATH=%APPVEYOR_BUILD_FOLDER%\dev\lib;%GOPATH%\bin;%Python2_ROOT_DIR%;%Python2_ROOT_DIR%\Scripts;%Python3_ROOT_DIR%;%Python3_ROOT_DIR%\Scripts;%MSYS_ROOT%\mingw64\bin;%MSYS_ROOT%\usr\bin\;%PATH%
- git clone --depth 1 https://github.com/datadog/integrations-core
- "%PIP2% install codecov -r requirements.txt"
- "%PIP3% install PyYAML==5.1"

cache:
- '%GOPATH%\bin'
Expand Down
9 changes: 7 additions & 2 deletions six/common/sixstrings.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,18 @@ PyObject *from_json(const char *data) {
goto done;
}

char module_name[] = "json";
// JSON return unicode strings under Python2, even if this is a better
// behavior it would be a breaking change compare to the previous version
// of the Agent not using this lib to embed Python. Yaml returns bytes
// under Python2 and Unicode under Python3. Since JSON is a subset of Yaml
// we use 'yaml' here.
char module_name[] = "yaml";
Copy link
Member

@arbll arbll May 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

JSON is indeed a subset of YAML 1.2. However I am not 100% sure if pyyaml implements 1.2 guidelines:
yaml/pyyaml#116

YAML 1.2 changes were mainly about dealing with some corner case that made YAML "kind of" a superset of JSON. diff are highlighted here: https://github.com/yaml/yaml/wiki/YAML-1.2-Changelog. I think there is a chance we might hit some of them. WDYT ?

Copy link
Member Author

@hush-hush hush-hush May 14, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

We completely control the data between the agent and six. It's serialized by the agent and loaded by six so we know that escaped / would not happened (which is the only one we care about).

But you raise a good point, since we control the whole chain why not use YAML everywhere !

I open another PR (#3468) with the whole change.

@masci @arbll Tell me what you think between the two.

json = PyImport_ImportModule(module_name);
if (json == NULL) {
goto done;
}

char func_name[] = "loads";
char func_name[] = "safe_load";
loads = PyObject_GetAttrString(json, func_name);
if (loads == NULL) {
goto done;
Expand Down