Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Added pflag for stow config in storage #95

Merged
merged 3 commits into from
Aug 13, 2021
Merged

Conversation

yindia
Copy link
Contributor

@yindia yindia commented Aug 12, 2021

TL;DR

Currently we are not generating pflag for stow config that means we are only able to pass sandbox storage config via flags.

  • Added pflag for stow config
# New flags 
$  --stow.kind S3 --stow.config key=value,key2=val2

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

Remove the 'fixes' keyword if there will be multiple PRs to fix the linked issue

fixes https://github.com/flyteorg/flyte/issues/

Follow-up issue

NA
OR
https://github.com/flyteorg/flyte/issues/

EngHabu
EngHabu previously approved these changes Aug 12, 2021
Copy link
Contributor

@EngHabu EngHabu left a comment

Choose a reason for hiding this comment

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

Thank you!

Signed-off-by: Yuvraj <[email protected]>
@yindia
Copy link
Contributor Author

yindia commented Aug 12, 2021

@EngHabu Do you know why test are failing ?

Signed-off-by: Yuvraj <[email protected]>
@yindia yindia force-pushed the feature/storage-pflag branch from 22e6a5d to 3ac2955 Compare August 13, 2021 03:23
@codecov
Copy link

codecov bot commented Aug 13, 2021

Codecov Report

Merging #95 (3ac2955) into master (9f637fd) will decrease coverage by 0.30%.
The diff coverage is 44.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #95      +/-   ##
==========================================
- Coverage   66.84%   66.53%   -0.31%     
==========================================
  Files          59       59              
  Lines        2817     2824       +7     
==========================================
- Hits         1883     1879       -4     
- Misses        789      799      +10     
- Partials      145      146       +1     
Flag Coverage Δ
unittests 64.89% <50.00%> (-0.35%) ⬇️

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

Impacted Files Coverage Δ
storage/config.go 60.00% <ø> (ø)
storage/config_flags.go 48.64% <37.50%> (-4.69%) ⬇️
storage/stow_store.go 73.01% <100.00%> (ø)
cache/auto_refresh.go 77.67% <0.00%> (-6.25%) ⬇️
sets/generic_set.go 94.56% <0.00%> (+1.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f637fd...3ac2955. Read the comment docs.

@yindia yindia requested a review from EngHabu August 13, 2021 03:29
@EngHabu EngHabu merged commit f6f76d8 into master Aug 13, 2021
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
* Generated pflag for stow config

Signed-off-by: Yuvraj <[email protected]>

* more changes

Signed-off-by: Yuvraj <[email protected]>

* more changes

Signed-off-by: Yuvraj <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants