Update runtime::container::start() to take a new allow_local flag
#1361
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 should fix an issue encountered by people running a local Flow stack under WSL. The problem is that in that case, Docker containers run in a separate VM to the one hosting the Linux VM, meaning that the container IP address exposed via `docker inspect` is not accessible from the "host". The solution here is to ask Docker to bind a port on the host to the corresponding port inside the container. In order to prevent binding every task to a host port in production (which runs Linux), we ended up turning off host-port mapping on Linux entirely because we didn't have a better way to identify local vs prod environments. We have since introduced an `allow_local` flag, and can condition host-port mapping on that, enabling Flow to run inside WSL.
github-actions bot
pushed a commit
to estuary/homebrew-flowctl
that referenced
this pull request
May 31, 2024
## What's Changed * `sum` annotation now supports arbitrary precision using string-encoded numerics * Add experimental `flowctl raw stats` sub-command * Various minor JSON Schema handling improvements. * Switch to simd-json for fast JSON parsing and transcoding. ### Filtered PRs impacting `flowctl`: * crates/json: don't validate strings with underscores as integers or numbers by @williamhbaker in estuary/flow#1364 * Update `runtime::container::start()` to take a new `allow_local` flag by @jshearer in estuary/flow#1361 * json: fix ordering of integers greater than i64::MAX by @psFried in estuary/flow#1367 * validation: fix bucket name validation for GCS and Azure by @psFried in estuary/flow#1370 * thread through `--allow-local` argument when running locally by @psFried in estuary/flow#1374 * validation: allow unsatisfiable constraints on excluded fields by @psFried in estuary/flow#1375 * update a number of dependencies, including RocksDB (to 8.10) by @jgraettinger in estuary/flow#1389 * connector-init: set connector_type on protocol check Spec by @jgraettinger in estuary/flow#1400 * models/journals: region configuration for S3 storage mappings by @williamhbaker in estuary/flow#1410 * improve schema validation errors by including metadata about the collection that failed by @jgraettinger in estuary/flow#1408 * flowctl: resurrect stats subcommand under raw by @psFried in estuary/flow#1432 * make: codesign binaries on mac by @mdibaiee in estuary/flow#1436 * simd-doc, gazette, avro, and dekaf crates by @jgraettinger in estuary/flow#1448 * flowctl(preview): multiple bindings may read from one collection by @mdibaiee in estuary/flow#1466 * crates/doc: support arbitrary precision with `sum` annotation by @jgraettinger in estuary/flow#1477 * crates/doc: relax `sum` inspection to allow numeric strings by @jgraettinger in estuary/flow#1481 **Full Changelog**: estuary/flow@v0.3.12...v0.3.13
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.
Description:
This should fix an issue encountered by people running a local Flow stack under WSL. The problem is that in that case, Docker containers run in a separate VM to the one hosting the Linux VM, meaning that the container IP address exposed via
docker inspectis not accessible from the "host". The solution here is to ask Docker to bind a port on the host to the corresponding port inside the container.In order to prevent binding every task to a host port in production (which runs Linux), we ended up turning off host-port mapping on Linux entirely because we didn't have a better way to identify local vs prod environments.
We have since introduced an
allow_localflag, and can condition host-port mapping on that, enabling Flow to run inside WSL.Notes for reviewers:
I did also add a debug log so we can see the real
docker runinvocation. This might be noisy (but also, if you turn on debug logging noisy is kind of expected), but it's something I've reached for every time I've had to debug why this piece of code isn't working.This change is