fix: uses_request being applied to uses_after and uses_before

[JohannesMessner]

No description provided.

[codecov]

Codecov Report

Merging #4228 (93b5f73) into master (5cef964) will increase coverage by 5.43%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #4228      +/-   ##
==========================================
+ Coverage   81.38%   86.82%   +5.43%     
==========================================
  Files         114      114              
  Lines        8272     8286      +14     
==========================================
+ Hits         6732     7194     +462     
+ Misses       1540     1092     -448     

Flag	Coverage Δ
daemon	42.43% <0.00%> (+0.13%)	⬆️
jina	86.56% <100.00%> (+8.85%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
jina/orchestrate/pods/__init__.py	80.80% <100.00%> (+3.47%)	⬆️
jina/serve/runtimes/gateway/http/app.py	90.80% <0.00%> (-0.21%)	⬇️
jina/serve/runtimes/gateway/http/models.py	99.10% <0.00%> (-0.04%)	⬇️
jina/jaml/__init__.py	95.47% <0.00%> (+0.41%)	⬆️
jina/orchestrate/pods/config/docker_compose.py	100.00% <0.00%> (+0.53%)	⬆️
jina/serve/networking.py	71.27% <0.00%> (+1.08%)	⬆️
jina/helper.py	81.87% <0.00%> (+1.22%)	⬆️
jina/jaml/parsers/flow/v1.py	98.24% <0.00%> (+1.75%)	⬆️
jina/clients/base/http.py	96.22% <0.00%> (+2.10%)	⬆️
jina/jaml/helper.py	85.36% <0.00%> (+2.43%)	⬆️
... and 13 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fce3ce7...93b5f73. Read the comment docs.

[github-actions]

Latency summary

Current PR yields:

• 🐢🐢 index QPS at 874, delta to last 2 avg.: -42%
• 🐎🐎🐎🐎 query QPS at 43, delta to last 2 avg.: +39%
• 🐢🐢 avg flow time within 1.2883 seconds, delta to last 2 avg.: -6%
• 😶 import jina within 0.3552 seconds, delta to last 2 avg.: +1%

Breakdown

Version	Index QPS	Query QPS	Avg Flow Time (s)	Import Time (s)
current	874	43	1.2883	0.3552
2.6.4	1583	30	1.4619	0.3519
2.6.3	1477	31	1.2809	0.3448

Backed by latency-tracking. Further commits will update this comment.

[cristianmtr]

Looks good, but is the bar available on /bar? Can you add a test to assert this?

[JohannesMessner]

No; should it be?

def test_override_requests_two_entrypoints():
    class FooExecutor(Executor):
        @requests(on='/bar')
        def foo(self, docs, **kwargs):
            for doc in docs:
                doc.text = 'foo called'

    with Flow(port_expose=exposed_port).add(
        uses=FooExecutor,
        uses_requests={'/foo': 'foo'},
    ) as f:
        c = Client(port=exposed_port)
        resp1 = c.post(
            on='/foo', inputs=DocumentArray([Document(text='')]), return_results=True
        )
        resp2 = c.post(
            on='/bar',
            inputs=DocumentArray([Document(text='')]),
            return_results=True,
        )

    assert resp1[0].docs[0].text == 'foo called'
    assert resp2[0].docs[0].text == 'foo called'

This fails, but that shouldn't have anything to do with our change.

[cristianmtr]

No, all good for this PR, from what I can tell. Just wondering in terms of general usage. I would expect that OtherExecutor still has its endpoint bound to /bar

[JohannesMessner]

I agree, might be worth a change

[JohannesMessner]

Re-read your comment, understood it this time, and added a test accordingly ;)

[cristianmtr]

Oh so it does preserve the OtherExecutor.bar binding. Good

[samsja]

related to this issue #4054