-
Notifications
You must be signed in to change notification settings - Fork 0
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
DM-45676: Update repo infrastructure #160
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This takes care of installing nox and caching.
We now recompute dependencies whenever we update the app, rather than on an automatic schedule. periodic-ci.yaml ensures that the dependencies can be updated without breaking tests.
This toml configuration comes from the FastAPI template at https://github.com/lsst/templates
This updates the Pydantic dependency to version 2, but likely isn't working yet. The issue is that Kafkit is still using Pydantic 2. But since we're moving to faststream and using Pydantic models directly without the Confluent Schema Registry we may just switch to that now too.
This switches Ook to using faststream and directly using Pydantic models rather than using Kafkit's Pydantic adapter for the Confluent Schema Registry. This faststream is approach is also how we're now building squarebot.
To get testcontainers to work - Change the app so that its creation is deferred to a create_app function - Don't import the kafka_router into the main namespace so that the app fixture in conftest.py can reset the kafka_router with the new configuration.
This changes the approach previously taken where I ran testcontainers in the pytest conftest.py module. Here I'm running the testcontainers in the nox sessions, which is more similar to how tox-docker runs. The advantage of this approach is that it doesn't require any code modifications and patching because the environment variables for the Kafka configuration can be made *before* starting up the app tests / development run or documentation building sessions. Running testcontainers via pytest could still be a useful technique if different tests need full isolation of their testcontainers, but of course this is a slow and resource-intensive approach.
We need to make sure testcontainers[kafka] is in the environment that nox is running in. Locally this is handled by having nox set up the local virtual environmemnt with nox -s init or nox -s venv-init.
jonathansick
force-pushed
the
tickets/DM-45676
branch
from
August 13, 2024 18:18
504f45b
to
9a2eb13
Compare
jonathansick
added a commit
to lsst-sqre/phalanx
that referenced
this pull request
Aug 13, 2024
Updates Ook to use faststream for its kafka usage. See lsst-sqre/ook#160
jonathansick
force-pushed
the
tickets/DM-45676
branch
from
August 13, 2024 19:24
9104bf0
to
941ba82
Compare
This is needed to ensure the standalone Factory in the CLI context works.
This ensures that the handler function is imported and therefore registered to the kafka router. Note need to avoid circ deps with kafkarouter
jonathansick
force-pushed
the
tickets/DM-45676
branch
from
August 14, 2024 20:27
4ae7a37
to
1bac26f
Compare
jonathansick
added a commit
to lsst-sqre/phalanx
that referenced
this pull request
Aug 14, 2024
Updates Ook to use faststream for its kafka usage. See lsst-sqre/ook#160 Includes an updated Kafka SSL configuration: - Update environment variable names to match the new Ook configuration models. - Drop the Confluent Schema Registry configurations, which are no longer needed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ruff-shared.toml
.