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.

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

Fix tagger regression behavior with SIX #3767

Merged
merged 1 commit into from
Jul 9, 2019

Conversation

clamoriniere
Copy link
Contributor

What does this PR do?

Previously in a python check, a call to the tagger.tag function, always
return a list (empty or not). With the introduction of SIX, in case of
the absence of associated tag stored in the tagger, the function returns
"NONE" that can break some checks (Kubelet check of instance).

This fix restores the previous behavior.

Motivation

What inspired you to submit this pull request?

Additional Notes

Anything else we should know when reviewing?

@clamoriniere clamoriniere requested review from mfpierre and a team June 28, 2019 14:36
@arbll arbll modified the milestones: DCA_1.3.0, 6.12.1 Jun 28, 2019
six/common/builtins/tagger.c Outdated Show resolved Hide resolved
@clamoriniere clamoriniere force-pushed the clamoriniere/fixSixTagger branch 2 times, most recently from c3a1b17 to e3e97c9 Compare June 28, 2019 14:44
@clamoriniere clamoriniere requested a review from albertvaka June 28, 2019 14:45
albertvaka
albertvaka previously approved these changes Jun 28, 2019
Copy link
Contributor

@albertvaka albertvaka left a comment

Choose a reason for hiding this comment

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

Purrrfect 😸

Thanks for the fix!

Copy link
Member

@hush-hush hush-hush left a comment

Choose a reason for hiding this comment

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

could you add a releasenote ?

@clamoriniere
Copy link
Contributor Author

@hush-hush release note added

@clamoriniere clamoriniere force-pushed the clamoriniere/fixSixTagger branch from a7fe2f2 to 9bd399e Compare June 28, 2019 15:20
hush-hush
hush-hush previously approved these changes Jun 28, 2019
@olivielpeau olivielpeau modified the milestones: 6.12.1, 6.13.0 Jun 28, 2019
@albertvaka
Copy link
Contributor

albertvaka commented Jul 1, 2019

You got conflicts because we renamed six to rtloader (sorry, I really thought this had been merged earlier!). Can you rebase and ping me so I give you a ✅ again?

@clamoriniere clamoriniere force-pushed the clamoriniere/fixSixTagger branch 2 times, most recently from 8b61380 to 1384c6d Compare July 1, 2019 13:18
Previously in a python check, a call to the tagger.tag function, always
return a list (empty or not). With the introduction of rtloader , in case of
the absence of associated tag stored in the tagger, the function returns
"NONE" that can break some checks (Kubelet check of instance).
@clamoriniere clamoriniere force-pushed the clamoriniere/fixSixTagger branch from 1384c6d to 39c4c6a Compare July 1, 2019 13:56
@clamoriniere
Copy link
Contributor Author

@albertvaka rebased done :)

@clamoriniere clamoriniere removed the request for review from mfpierre July 1, 2019 16:37
@olivielpeau
Copy link
Member

🙇

(and btw, this also makes me realize that the extension should raise a python error instead of returning None when the extension hasn't been initialized properly with a callback, see for example: https://github.com/DataDog/datadog-agent/pull/3767/files#diff-4ba6351bb1a03c827838dd3215a4a085L89 ; I'll open a separate PR)

@albertvaka
Copy link
Contributor

@clamoriniere Should we merge this?

@clamoriniere
Copy link
Contributor Author

@albertvaka Yes we can now merge it.

@albertvaka albertvaka merged commit e233b0a into master Jul 9, 2019
@albertvaka albertvaka deleted the clamoriniere/fixSixTagger branch July 9, 2019 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants