logdog: support ecs variant #1042
Conversation
| .map(|(filename, command)| LogRequest { filename, command }) | ||
| } | ||
| } | ||
| else { |
There was a problem hiding this comment.
There's a lot of repetition here, and it's not the kind that's easy to visually compare and verify. Can we store common entries in a common table and append them to the variant-specific ones, maybe with chain or similar on the iterator?
| if let Ok(variant) = env::var("VARIANT") { | ||
| println!("cargo:rustc-cfg=variant=\"{}\"", variant); | ||
| for &variant_group in VARIANT_GROUPS { | ||
| if variant.starts_with(variant_group) { |
There was a problem hiding this comment.
I'd rather avoid the idea of variant groups/families until we have time for more thought and design on it. I think we can do a relatively simple change at the buildsys level that will enable this information for all builds, not just this one, and possibly with a better, more granular mechanism like features/labels for k8s, aws, etc. Or, at the least, move the decision out of the crate and into a place that already has to care about variant, for example using a data file to list the commands we run and installing a different data file per variant in the os package, or something similar. In my mind, this feature isn't critical for GA, and waiting is a better option than a one-off, given the complexity involved.
|
Closing in favor of a different approach. |
Issue number:
#815
Description of changes:
Conditionally compile a different set of
logdogcommands for ECS.Testing done:
Ran an ECS variant, got the ECS logs.
Ran a k8s variant, got the k8s logs.
** More work to be Done **
I wanted to get the low hanging fruit handled now, but I still need to do #1039 which requires a more substantial change in
logdog.Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.