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(dev): Unfeature flag internal_events::parser #18322

Closed
wants to merge 1 commit into from

Conversation

jszwedko
Copy link
Member

Was overeargerly flagged in #18308 which caused CI
failures in the nightly Component Features workflow.

See: https://github.com/vectordotdev/vector/actions/runs/5908210923/attempts/1

Signed-off-by: Jesse Szwedko jesse.szwedko@datadoghq.com

Was overeargerly flagged in #18308 which caused CI
failures in the nightly Component Features workflow.

See: https://github.com/vectordotdev/vector/actions/runs/5908210923/attempts/1

Signed-off-by: Jesse Szwedko <jesse.szwedko@datadoghq.com>
@netlify
Copy link

netlify bot commented Aug 21, 2023

Deploy Preview for vector-project ready!

Name Link
🔨 Latest commit 55ec249
🔍 Latest deploy log https://app.netlify.com/sites/vector-project/deploys/64e377f43a01060008f7eb6e
😎 Deploy Preview https://deploy-preview-18322--vector-project.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@netlify
Copy link

netlify bot commented Aug 21, 2023

Deploy Preview for vrl-playground canceled.

Name Link
🔨 Latest commit 55ec249
🔍 Latest deploy log https://app.netlify.com/sites/vrl-playground/deploys/64e377f450cd6400080a56e2

@datadog-vectordotdev
Copy link

Datadog Report

Branch report: jszwedko/fix-features
Commit report: ca2d1ef

vector: 0 Failed, 0 New Flaky, 1944 Passed, 0 Skipped, 1m 18.22s Wall Time

@hhromic
Copy link
Contributor

hhromic commented Aug 21, 2023

Ah, I saw now in the description of this PR which features failed.
Strange that this didn't trigger in the other PR's CI. Do you know why?

Anyway, I think the correct thing to do is not to unflag, but to flag the other components too to clearly represent the actual dependency of these components to the parser module. As I mentioned in the other PR, when compiling Vector with minimal flags, and the parser is not flagged, it will complain that there is unused code (the parser).

EDIT: The output of the actions job you linked is very useful to do this correctly actually. After this fixing PR is merged, I can work out better the feature flag dependencies 👍. I didn't know of make check-component-features before.

EDIT2: And apologies for breaking this 🙈.

@jszwedko
Copy link
Member Author

Replaced by #18327

@jszwedko jszwedko closed this Aug 21, 2023
@jszwedko
Copy link
Member Author

Strange that this didn't trigger in the other PR's CI. Do you know why?

That CI check only runs nightly because it is expensive and doesn't break very often. I should have thought to manually trigger it on your PR modifying feature flags though 😅

@jszwedko
Copy link
Member Author

EDIT2: And apologies for breaking this 🙈.

No worries! Normally CI would catch things like this.

@hhromic
Copy link
Contributor

hhromic commented Aug 21, 2023

That CI check only runs nightly because it is expensive and doesn't break very often. I should have thought to manually trigger it on your PR modifying feature flags though 😅

Ah that sounds like good insight to be careful running it locally :)
I will prepare a new PR with the feature flag re-added and I will add dependencies based on the job output you shared.

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.

3 participants