Skip to content
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

redesign config checker to support freeform keys #678

Merged
merged 9 commits into from
Apr 27, 2024

Conversation

dmachard
Copy link
Owner

@dmachard dmachard commented Apr 13, 2024

fix #676

Individual function IsValid has been added to the following structs:

  • Config
  • ConfigGlobal
  • ConfigCollectors
  • ConfigLoggers
  • ConfigMultiplexer
  • ConfigPipelines
  • ConfigTransformers

The config checker support freeform keys and the code is more easy to maintain!

@dmachard
Copy link
Owner Author

@pieterlexis-tomtom I would like to add you as reviewer but I cannot. Any feedback will be appreciated (if you can)

@dmachard dmachard changed the title add test to fix scalyr redesign config checker to support freeform keys Apr 19, 2024
@pieterlexis-tomtom
Copy link
Contributor

pieterlexis-tomtom commented Apr 24, 2024

I'll have a look, but I must say I am not well-versed with reflect

@dmachard
Copy link
Owner Author

No worries, or just some tests will be great too.

@pieterlexis-tomtom
Copy link
Contributor

This seems to work well on a few configs I tried 👍. Good stuff!

@pieterlexis-tomtom
Copy link
Contributor

If I can point to an improvement: You're reading the config file multiple times while loading (first for config, then checking), perhaps you should read it once and use the byte[] to create both the map[string]interface{} and Config struct? That ensures you're always working with the same data, should the file be changed underneath your feet.

@dmachard
Copy link
Owner Author

thanks, changed done!

@dmachard dmachard merged commit da4833b into main Apr 27, 2024
63 checks passed
@dmachard dmachard deleted the fix-checkconfig-scalyr branch May 7, 2024 19:24
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.

Configuration checks breaks freeform keys
2 participants