Skip to content

config: hopefully fixing C++ config default for apple#2355

Merged
alyssawilk merged 2 commits intoenvoyproxy:mainfrom
alyssawilk:apple
Jun 13, 2022
Merged

config: hopefully fixing C++ config default for apple#2355
alyssawilk merged 2 commits intoenvoyproxy:mainfrom
alyssawilk:apple

Conversation

@alyssawilk
Copy link
Contributor

@alyssawilk alyssawilk commented Jun 9, 2022 •
edited
Loading

Replacing the default dns resolver in the config string to be correct for the OS in question.
Replaced overrides in the swift code now that the base config handles it.

Risk Level: low
Testing: existing tests should verify correctness
Docs Changes: n/a
Release Notes: did not add

@alyssawilk
config: hopefully fixing C++ config default for apple
f3f8fa2 
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@jpsim
Copy link
Contributor

jpsim commented Jun 9, 2022

Can you also remove this line?

envoy-mobile/library/objective-c/EnvoyConfiguration.m

Line 157 in 2ea19b7

[definitions appendFormat:@"- &dns_resolver_name envoy.network.dns_resolver.apple\n"];

@alyssawilk
comments
4d52371 
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
@alyssawilk
Copy link
Contributor Author

alyssawilk commented Jun 13, 2022

Good call, thanks!

jpsim
jpsim approved these changes Jun 13, 2022
View reviewed changes
Copy link
Contributor

@jpsim jpsim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

goaway
goaway approved these changes Jun 13, 2022
View reviewed changes
Copy link
Contributor

@goaway goaway left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. Longer-term, I can see this being potentially useful to expose through the builder, but not as anything that would need to block this change. Thanks @alyssawilk!

@alyssawilk alyssawilk merged commit 521f38e into envoyproxy:main Jun 13, 2022
jpsim added a commit that referenced this pull request Jun 14, 2022
@jpsim
Merge remote-tracking branch 'origin/main' into jp-real-engine-handles
06b6753 
* origin/main: (33 commits)
  iOS: fix xcframework upload in release workflow (#2366)
  config: hopefully fixing C++ config default for apple (#2355)
  Update Envoy (#2364)
  Bump Lyft Support Rotation (#2365)
  ci: pin external GitHub Action (#2363)
  cleanup: fix warning in JNI layer (#2361)
  cleanup: convert some more uses of NULL to nullptr (#2359)
  cleanup: consistently use nullptr in cc contexts (#2351)
  cleanup: remove unused function and resolve warning (#2350)
  iOS: add configurable gzip and brotli decompression options (#2349)
  iOS: stop embedding bitcode in releases (#2347)
  ci: update Android setup (#2354)
  docs: update the list of clusters (#2344)
  bazel: update rules_apple (#2346)
  iOS: add a way to disable network monitoring (#2345)
  api: adding brotli knobs (#2342)
  android: create persistent SharedPreferences-based KV store (#2319)
  ios: add support for registering a platform KV store (#2334)
  builder: making compressor configurable (#2321)
  iOS: add SwiftPM example (#2333)
  ...

Signed-off-by: JP Simard <jp@jpsim.com>
jpsim added a commit that referenced this pull request Jun 14, 2022
@jpsim
Merge remote-tracking branch 'origin/main' into jp-real-engine-handles
32fcf13 
* origin/main: (33 commits)
  iOS: fix xcframework upload in release workflow (#2366)
  config: hopefully fixing C++ config default for apple (#2355)
  Update Envoy (#2364)
  Bump Lyft Support Rotation (#2365)
  ci: pin external GitHub Action (#2363)
  cleanup: fix warning in JNI layer (#2361)
  cleanup: convert some more uses of NULL to nullptr (#2359)
  cleanup: consistently use nullptr in cc contexts (#2351)
  cleanup: remove unused function and resolve warning (#2350)
  iOS: add configurable gzip and brotli decompression options (#2349)
  iOS: stop embedding bitcode in releases (#2347)
  ci: update Android setup (#2354)
  docs: update the list of clusters (#2344)
  bazel: update rules_apple (#2346)
  iOS: add a way to disable network monitoring (#2345)
  api: adding brotli knobs (#2342)
  android: create persistent SharedPreferences-based KV store (#2319)
  ios: add support for registering a platform KV store (#2334)
  builder: making compressor configurable (#2321)
  iOS: add SwiftPM example (#2333)
  ...

Signed-off-by: JP Simard <jp@jpsim.com>
@alyssawilk alyssawilk mentioned this pull request Jun 21, 2022
Closed
Augustyniak pushed a commit that referenced this pull request Jun 28, 2022
@alyssawilk @Augustyniak
config: hopefully fixing C++ config default for apple (#2355)
06ba881 
Replacing the default dns resolver in the config string to be correct for the OS in question.
Replaced overrides in the swift code now that the base config handles it.

Risk Level: low
Testing: existing tests should verify correctness
Docs Changes: n/a
Release Notes: did not add

Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Signed-off-by: Rafal Augustyniak <raugustyniak@lyft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jpsim jpsim jpsim approved these changes

@goaway goaway goaway approved these changes

Assignees

@goaway goaway

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alyssawilk @jpsim @goaway