close
Skip to content

fix(server): Fix overriding of config from env vars#2314

Merged
hubcio merged 1 commit into
apache:masterfrom
BrandonArp:fix_config_option
Nov 7, 2025
Merged

fix(server): Fix overriding of config from env vars#2314
hubcio merged 1 commit into
apache:masterfrom
BrandonArp:fix_config_option

Conversation

@BrandonArp
Copy link
Copy Markdown
Contributor

fix bug where overriding of s3 and file archiver config from environment variables was not working

fix bug where overriding of s3 and file archiver config from environment
variables was not working
@spetz
Copy link
Copy Markdown
Contributor

spetz commented Nov 6, 2025

Thanks for the contribution! Were you able to validate whether providing the default values allows the env override, or should we remove the Option<> from these fields?

@BrandonArp
Copy link
Copy Markdown
Contributor Author

BrandonArp commented Nov 6, 2025

It does work after the changes:
Running with
iggy/core/server$ IGGY_DATA_MAINTENANCE_ARCHIVER_ENABLED=true IGGY_DATA_MAINTENANCE_ARCHIVER_S3_KEY_ID=foo IGGY_DATA_MAINTENANCE_ARCHIVER_S3_BUCKET=foobucket IGGY_DATA_MAINTENANCE_ARCHIVER_S3_ENDPOINT=https://footendpoint IGGY_DATA_MAINTENANCE_ARCHIVER_DISK_PATH=bad/path cargo run

gives the config
Using Config: { data_maintenance: { archiver: { enabled: **true**, kind: disk, disk: { path: **bad/path** }, s3: { key_id: **foo**, bucket: **foobucket**, endpoint: **https://fooendpoint.** region: eu-west-1 } }, messages: { archiver_enabled: false, cleaner_enabled: false, interval: 1m }, state: { archiver_enabled: false, overwrite: true, interval: 1m } }, message_saver: { enabled: true, enforce_fsync: true, interval: 30s }, heartbeat: { enabled: false, interval: 5s }, system: { path: local_data, logging: { path: logs, level: info, max_size: 512.00 MB, retention: 7days }, stream: { path: streams }, topic: { path: topics, max_size: unlimited, delete_oldest_segments: false }, partition: { path: partitions, messages_required_to_save: 1024, size_of_messages_required_to_save: 1.05 MB, enforce_fsync: false, validate_checksum: false }, segment: { size_bytes: 1.07 GB, cache_indexes: OpenSegment, message_expiry: never_expire, archive_expired: false, server_confirmation: wait }, encryption: { enabled: false }, state: { enforce_fsync: false, max_file_operation_retries: 1, retry_delay: 1s } }, quic: { enabled: true, address: 127.0.0.1:8080, max_concurrent_bidi_streams: 10000, datagram_send_buffer_size: 100.00 KB, initial_mtu: 8.00 KB, send_window: 100.00 KB, receive_window: 100.00 KB, keep_alive_interval: 5s, max_idle_timeout: 10s, certificate: { self_signed: true, cert_file: core/certs/iggy_cert.pem, key_file: core/certs/iggy_key.pem } }, tcp: { enabled: true, address: 127.0.0.1:8090, ipv6: false, tls: { enabled: false, cert_file: core/certs/iggy_cert.pem, key_file: core/certs/iggy_key.pem }, socket: { override defaults: false, recv buffer size: 100.00 KB, send buffer size 100.00 KB, keepalive: false, nodelay: false, linger: 0s } }, http: { enabled: true, address: 127.0.0.1:3000, max_request_size: 2.00 MB, cors: { enabled: true, allowed_methods: ["GET", "POST", "PUT", "DELETE"], allowed_origins: ["*"], allowed_headers: ["content-type"], exposed_headers: [""], allow_credentials: false, allow_private_network: false }, jwt: { algorithm: HS256, audience: iggy.apache.org, access_token_expiry: 1h, use_base64_secret: false }, metrics: { enabled: true, endpoint: /metrics }, tls: { enabled: false, cert_file: core/certs/iggy_cert.pem, key_file: core/certs/iggy_key.pem } }, telemetry: { enabled: false, service_name: iggy, logs: { transport: grpc, endpoint: http://localhost:7281/v1/logs }, traces: { transport: grpc, endpoint: http://localhost:7281/v1/traces } } }

I am not sure why the Option<> wrapping was done and didn't feel comfortable removing it without knowing why it was done. If you'd like me to, I can do that, though.

@spetz
Copy link
Copy Markdown
Contributor

spetz commented Nov 7, 2025

Ok, no need to remove Option<> then, we'll probably need to improve our custom config provider :)

@hubcio hubcio merged commit 9ae08f9 into apache:master Nov 7, 2025
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants