Skip to content

Add http config options#132

Merged
mimir-d merged 1 commit intolinuxboot:mainfrom
mahmednabil109:add_httplogger_options
Aug 17, 2022
Merged

Add http config options#132
mimir-d merged 1 commit intolinuxboot:mainfrom
mahmednabil109:add_httplogger_options

Conversation

@mahmednabil109
Copy link
Copy Markdown
Contributor

@mahmednabil109 mahmednabil109 commented Aug 16, 2022

Signed-off-by: Mohamed Abokammer mahmednabil109@gmail.com

@mahmednabil109 mahmednabil109 changed the title Add http options Add http config options Aug 16, 2022
Comment thread pkg/loggerhook/httphook.go Outdated
flagInstanceTag *string
flagLogLevel *string
flagAdminServerAddr *string
flagHttpLoggerBufferSize *int
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is there a way to separate these in another set or something? seems like a lot of noise for the main server flags and not much value, since defaults are almost always acceptable

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is it possible to use more than one set?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yes and no; it would be a bit difficult with this flags package, so id leave it as is for now

Comment thread cmds/contest/server/server.go Outdated
Comment thread pkg/xcontext/bundles/options.go Outdated
Comment thread pkg/xcontext/bundles/options.go Outdated
@mahmednabil109 mahmednabil109 marked this pull request as ready for review August 17, 2022 10:45
- Add options to override http logger defaults from the cli

Signed-off-by: Mohamed Abokammer <mahmednabil109@gmail.com>
@mahmednabil109 mahmednabil109 force-pushed the add_httplogger_options branch from 90c1690 to 087509f Compare August 17, 2022 12:43
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Aug 17, 2022

Codecov Report

Merging #132 (8ed27c8) into main (fe65fea) will decrease coverage by 0.08%.
The diff coverage is 17.85%.

❗ Current head 8ed27c8 differs from pull request most recent head 087509f. Consider uploading reports for the commit 087509f to get more accurate results

@@            Coverage Diff             @@
##             main     #132      +/-   ##
==========================================
- Coverage   63.61%   63.52%   -0.09%     
==========================================
  Files         164      164              
  Lines       10330    10346      +16     
==========================================
+ Hits         6571     6572       +1     
- Misses       3036     3052      +16     
+ Partials      723      722       -1     
Flag Coverage Δ
e2e 49.40% <17.85%> (-0.04%) ⬇️
integration 54.84% <0.00%> (-0.09%) ⬇️
unittests 49.31% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/loggerhook/httphook.go 0.00% <0.00%> (ø)
pkg/xcontext/bundles/logrusctx/new_context.go 66.00% <0.00%> (ø)
pkg/xcontext/bundles/options.go 57.89% <0.00%> (ø)
cmds/contest/server/server.go 56.66% <31.25%> (-2.13%) ⬇️
pkg/runner/step_state.go 88.57% <0.00%> (-1.91%) ⬇️
pkg/jobmanager/jobmanager.go 76.92% <0.00%> (-1.10%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mimir-d mimir-d merged commit c0c55f0 into linuxboot:main Aug 17, 2022
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.

Override Default values in http logger

4 participants