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
WIP: [AIRFLOW-4506] Make GIT_SYNC_DEPTH and GIT_SYNC_REV configurable #5281
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5281 +/- ##
==========================================
+ Coverage 78.76% 78.76% +<.01%
==========================================
Files 474 474
Lines 30142 30144 +2
==========================================
+ Hits 23741 23743 +2
Misses 6401 6401
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## master #5281 +/- ##
==========================================
+ Coverage 78.76% 78.76% +<.01%
==========================================
Files 474 474
Lines 30142 30144 +2
==========================================
+ Hits 23741 23743 +2
Misses 6401 6401
Continue to review full report at Codecov.
|
Hey @dimberman I'm not sure whether this is the correct approach to default values in this case as I want to keep it backward compatible (meaning no entries in |
Hi @andrejbaran I'm not sure what the question is? This seems in-line with existing stuff. That said, could you add a test for this please? |
@@ -64,10 +64,13 @@ def _get_init_containers(self): | |||
'value': self.kube_config.git_sync_dest | |||
}, { | |||
'name': 'GIT_SYNC_DEPTH', | |||
'value': '1' | |||
'value': self.kube_config.git_sync_depth if self.kube_config.git_sync_depth else '1' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't' put the default in code, just put the default in the config file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move defaults for both new config entries out of the code into the config file.
@andrejbaran any word on this? Would love to get it merged ASAP :). |
@andrejbaran Can I help you with this change? |
@mik-laj @dimberman hey guys, sorry for not responding. I will try to finish this up next week. |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Make sure you have checked all steps below.
Jira
Description
Tests
Commits
Documentation
Code Quality
flake8