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

docs: make sure that channels use finally in all examples #3796

Merged
merged 2 commits into from
Oct 15, 2024

Conversation

sobolevn
Copy link
Member

Right now this code:
Снимок экрана 2024-10-14 в 18 20 32

Is not really correct, because in case of an exception - it will not execute await channels.unsubscribe(subscriber, ["foo"]), which might be unexpected.

There's a similar example above that use the correct form:

Снимок экрана 2024-10-14 в 18 21 35

@sobolevn sobolevn requested review from a team as code owners October 14, 2024 15:22
@github-actions github-actions bot added area/docs This PR involves changes to the documentation size: small type/docs pr/external Triage Required 🏥 This requires triage labels Oct 14, 2024
@sobolevn
Copy link
Member Author

This failure does not look related: https://github.com/litestar-org/litestar/actions/runs/11330520189/job/31508525457?pr=3796

==================================== ERRORS ====================================
_______________ ERROR at setup of test_with_stores[redis_store] ________________
[gw0] linux -- Python 3.12.7 /home/runner/work/litestar/litestar/.venv/bin/python

request = <SubRequest 'store' for <Coroutine test_with_stores[redis_store]>>

    @pytest.fixture(
        params=[pytest.param("redis_store", marks=pytest.mark.xdist_group("redis")), "memory_store", "file_store"]
    )
    def store(request: FixtureRequest) -> Store:
>       return cast("Store", request.getfixturevalue(request.param))

tests/conftest.py:111: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
.venv/lib/python3.12/site-packages/pytest_asyncio/plugin.py:369: in _async_fixture_wrapper
    return event_loop.run_until_complete(setup())
/opt/hostedtoolcache/Python/3.12.7/x64/lib/python3.12/asyncio/base_events.py:687: in run_until_complete
    return future.result()
.venv/lib/python3.12/site-packages/pytest_asyncio/plugin.py:364: in setup
    res = await func(
tests/docker_service_fixtures.py:127: in redis_service
    await docker_services.start("redis", check=redis_responsive)
tests/docker_service_fixtures.py:80: in start
    self.run_command("up", "-d", name)
tests/docker_service_fixtures.py:68: in run_command
    subprocess.run(command, check=True, capture_output=True)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

input = None, capture_output = True, timeout = None, check = True
popenargs = (['docker', 'compose', '--file=/home/runner/work/litestar/litestar/tests/docker-compose.yml', '--project-name=litestar_pytest-gw0', 'up', '-d', ...],)
kwargs = {'stderr': -1, 'stdout': -1}
process = <Popen: returncode: 18 args: ['docker', 'compose', '--file=/home/runner/work...>
stdout = b''
stderr = b'time="2024-10-14T15:24:33Z" level=warning msg="/home/runner/work/litestar/litestar/tests/docker-compose.yml: `versio...l rate limit. You may increase the limit by authenticating and upgrading: [https://www.docker.com/increase-rate-limit\n](https://www.docker.com/increase-rate-limit/n)'
retcode = 18

@sobolevn sobolevn enabled auto-merge (squash) October 15, 2024 07:26
@sobolevn sobolevn disabled auto-merge October 15, 2024 07:26
@sobolevn
Copy link
Member Author

@Alc-Alc can you please take a look? :)

Copy link

Copy link

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/3796

@Alc-Alc
Copy link
Contributor

Alc-Alc commented Oct 15, 2024

@Alc-Alc can you please take a look? :)

finally did 😅 thank you 🙂

@Alc-Alc Alc-Alc merged commit 173098b into main Oct 15, 2024
26 checks passed
@Alc-Alc Alc-Alc deleted the channels-use-finally branch October 15, 2024 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docs This PR involves changes to the documentation pr/external size: small Triage Required 🏥 This requires triage type/docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants