fix-read-timout-dispatcher

[intojhanurag]

Propagates the ReadTimeout option from InMemoryEventDispatcherArgs to
the underlying HTTPEventReceiver. The dispatcher previously created a
receiver without setting its ReadTimeout, despite the TODO in the code.

[knative-prow]

Hi @intojhanurag. Thanks for your PR.

I'm waiting for a github.com member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[intojhanurag]

Hey @Cali0707 , Do you think , Is it good to go? Please take a look if you have a moment.

[Cali0707]

Hey @intojhanurag - this seems to be the same change as you proposed in #8786 originally. In general, when making updates to a set of changes you've already submitted as a PR you should update the existing PR rather than opening a new one. That helps everyone understand the history and context behind the change

[Cali0707]

Thanks @intojhanurag for this - this looks good overall.

I have one small nit on the code itself, otherwise just keep in mind my comment about trying to keep changes in the same PR and not opening new ones in the future 😄

[Cali0707]

/ok-to-test

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.11%. Comparing base (161a91a) to head (a75fa64).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8790   +/-   ##
=======================================
  Coverage   51.10%   51.11%           
=======================================
  Files         409      409           
  Lines       21356    21360    +4     
=======================================
+ Hits        10915    10919    +4     
  Misses       9589     9589           
  Partials      852      852           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[intojhanurag]

My bad 😭! I reopened a new PR because the previous one slipped my mind. While exploring the repo again, I noticed the same issue and ended up creating a new PR unintentionally.

[intojhanurag]

Hey @creydr , Can you review this one please ?

[creydr]

Thanks for working on this @intojhanurag

I only left a tiny comment

[intojhanurag]

Hey @creydr , Please can you point out why these tests are failing after new changes ?

[creydr]

Hey @creydr , Please can you point out why these tests are failing after new changes ?

Its a build issue. The code doesn't compile. Seems #8790 (comment) was too simple. We probably need something like

	bindingsReceiver := kncloudevents.NewHTTPEventReceiver(
		args.Port,
		append(
			[]kncloudevents.HTTPEventReceiverOption{
				kncloudevents.WithReadTimeout(args.ReadTimeout),
			},
			args.HTTPEventReceiverOptions...)...,
	)

[intojhanurag]

Hey @creydr , Now Is it good to go ? Again one test is failing

[creydr]

/retest-required

[creydr]

/test reconciler-tests

[creydr]

/test reconciler-tests

[creydr]

/lgtm

[knative-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: creydr, intojhanurag

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• OWNERS [creydr]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[creydr]

/retest-required

[intojhanurag]

Thanks @creydr

[creydr]

Finally 😄 Thanks @intojhanurag for your patience!