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

[aws] Remove unused function newV2Context from input_test.go #39538

Merged
merged 2 commits into from
May 15, 2024

Conversation

kaiyan-sheng
Copy link
Contributor

Proposed commit message

This PR is to remove the unused function newV2Context from input_test.go. This function also is already declared in input_integration_test.go. This should fix error in buildkite:

=== Errors
input/awss3/input_test.go:159:6: newV2Context redeclared in this block
	input/awss3/input_integration_test.go:147:6: other declaration of newV2Context
input/awss3/input_integration_test.go:138:46: undefined: s3Input
input/awss3/input_integration_test.go:144:19: undefined: s3Input
input/awss3/input_integration_test.go:238:14: undefined: s3Input
DONE 1369 tests, 15 skipped, 4 errors in 113.021s
Error: failed to execute go: exit status 1

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have made corresponding change to the default configuration files
  • I have added tests that prove my fix is effective or that my feature works
  • I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

@kaiyan-sheng kaiyan-sheng requested a review from a team as a code owner May 13, 2024 22:15
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label May 13, 2024
Copy link
Contributor

mergify bot commented May 13, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @kaiyan-sheng? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@kaiyan-sheng kaiyan-sheng added the Team:obs-ds-hosted-services Label for the Observability Hosted Services team label May 14, 2024
@elasticmachine
Copy link
Collaborator

Pinging @elastic/obs-ds-hosted-services (Team:obs-ds-hosted-services)

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label May 14, 2024
@kaiyan-sheng kaiyan-sheng merged commit 0ac96f4 into elastic:main May 15, 2024
18 of 19 checks passed
@kaiyan-sheng kaiyan-sheng deleted the fix_input_test branch May 15, 2024 14:46
dliappis added a commit to dliappis/beats that referenced this pull request May 21, 2024
@dliappis
Copy link
Contributor

@kaiyan-sheng unfortunately the checks triggered by this PR didn't trigger the failing test described in the issue because it's an optional test that gets triggered only when the aws GH label is present (see https://github.com/elastic/beats/blob/main/.buildkite/x-pack/pipeline.xpack.filebeat.yml#L264).
So I raised a new (throwaway) PR #39647 from the latest main, which includes this commit but it appears the test still fails: https://buildkite.com/elastic/beats-xpack-filebeat/builds/2863#018f9b86-bb0e-4f13-888a-a8d23e752f06/168-1517

Could you please take another look at it?

@dliappis
Copy link
Contributor

dliappis commented May 21, 2024

@kaiyan-sheng looking at it a bit more with the help of @pkoutsovasilis it seems to me that this PR attempted to fix only the first error:

=== Errors
input/awss3/input_test.go:159:6: newV2Context redeclared in this block
	input/awss3/input_integration_test.go:147:6: other declaration of newV2Context

Looking at a test CI run including the commit it appears that the above error got fixed, yay!

Btw newV2Context was added as part of #39353, which never got backported, therefore, this PR is ok to not be backported too.

@pkoutsovasilis raised a PR #39648 to fix the other s3input errors:

input/awss3/input_integration_test.go:138:46: undefined: s3Input
input/awss3/input_integration_test.go:144:19: undefined: s3Input
input/awss3/input_integration_test.go:238:14: undefined: s3Input

so we can move the discussion there. Thanks again.

@kaiyan-sheng
Copy link
Contributor Author

@dliappis ahh yes! Thank you @pkoutsovasilis for the PR to fix the s3input error!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team:obs-ds-hosted-services Label for the Observability Hosted Services team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants