You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/09/01 14:41:16 UTC
[GitHub] [airflow] kiwy42 opened a new pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
kiwy42 opened a new pull request #17816:
URL: https://github.com/apache/airflow/pull/17816
Feature request #17314
Add to default option in `config.yaml` and `default_airflow.cfg` to cause no regression.
kinit can now emit non forwardable ticket and ticket without originate IP.
Add 2 default configuration value:
[kerberos]
forwardable = True (this one will trigger kinit argument -f or -F)
include_ip = True ( this one will trigger kinit argument -a or -A)
Edit kerberos.py to enable usage of new argument [-f | -F] and [-a | -A]
From old pull request #17342
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 closed pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 closed pull request #17816:
URL: https://github.com/apache/airflow/pull/17816
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-906971194
How frustrating, seems like things have been failing out of API issue...
So disappointed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb merged pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
ashb merged pull request #17816:
URL: https://github.com/apache/airflow/pull/17816
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-906434277
> Is it OK to have spelling mistake ?
Nope. if a word is technical and is not available in our dictionary, you can add it to custom dictionary. https://github.com/apache/airflow/blob/main/docs/spelling_wordlist.txt
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] boring-cyborg[bot] commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
boring-cyborg[bot] commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-910502088
Awesome work, congrats on your first merged pull request!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-907519079
>
>
> Yep. There was GitHub Packages incident: https://www.githubstatus.com/incidents/wsh4dvm6x81k closed/reopened to rebuild.
OK wasn't sure if it was coming from me or not, however most MSSql integration did fail and I'm pretty sure it's not what I did that influence the issue, is it normal ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-905004300
Can you add docs for these options?https://airflow.apache.org/docs/apache-airflow/stable/security/kerberos.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-907527819
Not your fault for MSSQL (Something we need to fix) but the static check is definitely yours to fix ;).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-906171952
@potiuk I've tried to add some tests, but my knowledge on how to verify if option is effective or not is limited so I probably won't be able to do better than that in `test/security/kerberos.py`:
```
# Validate forwardable kerberos option
@conf_vars({('kerberos', 'keytab'): KRB5_KTNAME,('kerberos','forwardable'): ''})
def test_renew_from_kt_forwardable_empty(self):
"""
We expect no result, but a successful run.
"""
assert renew_from_kt(principal=self.args.principal, keytab=self.args.keytab) is None
@conf_vars({('kerberos', 'keytab'): KRB5_KTNAME,('kerberos','forwardable'): 'False'})
def test_renew_from_kt_forwardable_false(self):
"""
We expect no result, but a successful run.
"""
assert renew_from_kt(principal=self.args.principal, keytab=self.args.keytab) is None
@conf_vars({('kerberos', 'keytab'): KRB5_KTNAME,('kerberos','forwardable'): 'True'})
def test_renew_from_kt_forwardable_true(self):
"""
We expect no result, but a successful run.
"""
assert renew_from_kt(principal=self.args.principal, keytab=self.args.keytab) is None
```
It's nothing too fancy but at least validate that my option do not disrupt regular operation.
@mik-laj I will try to edit documentation accordingly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-906424944
I've seen an error in documentation build I missed a jumb line and still learning how to use breeze to develop on Airflow, but still, I'm using the term `forwardable` as my option and in documentation and `forwardability` which are both term that are more technical than proper British English. However when using `kinit -h` one can read:
```
kinit -h
kinit: invalid option -- 'h'
Usage: kinit [-V] [-l lifetime] [-s start_time] [-r renewable_life]
[-f | -F] [-p | -P] [-n] [-a | -A] [-C] [-E]
[--request-pac | --no-request-pac]
[-v] [-R] [-k [-i|-t keytab_file]] [-c cachename]
[-S service_name] [-I input_ccache] [-T ticket_armor_cache]
[-X <attribute>[=<value>]] [principal]
options:
[...]
-f forwardable
-F not forwardable
[...]
-a include addresses
-A do not include addresses
-v validate
[...]
```
Is it OK to have spelling mistake ?
I can push a documentation correction to allow proper build but I do not know how I could not use the term `forwardable`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-909094402
OK, now I'm over 9000 frustrated, and this error:
```
ERROR: Cannot install apache-airflow-providers-celery==2.0.0.dev0 because these package versions have conflicting dependencies.
The conflict is caused by:
apache-airflow-providers-celery 2.0.0.dev0 depends on celery>=5.1.2 and ~=5.1
The user requested (constraint) celery==4.4.7
```
Is the reason why I stopped trying to build Airflow, dependencies constraints broke so easily, I don't know how or why but this made me stop trying to build anything myself.
Hope I will one day finish this pull request :D
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-908229518
> Not your fault for MSSQL (Something we need to fix) but the static check is definitely yours to fix ;).
I'm trying but those pre commit static checks I don''t understand, it compares my file with my previous file but they changed.
I'm a bit puzzled.
Also there's those fail that do not create an explicit error:
```
black....................................................................................Failed
- hook id: black
- files were modified by this hook
reformatted airflow/security/kerberos.py
reformatted tests/security/test_kerberos.py
All done! ✨ 🍰 ✨
2 files reformatted, 2725 files left unchanged.
All done! ✨ 🍰 ✨
244 files left unchanged.
```
and
```
Checks for consistency between config.yml and default_config.cfg.........................Failed
- hook id: airflow-config-yaml
- files were modified by this hook
Converting ./scripts/ci/pre_commit/../../../airflow/config_templates/config.yml to ./scripts/ci/pre_commit/../../../airflow/config_templates/default_airflow.cfg
Converting ./scripts/ci/pre_commit/../../../airflow/providers/google/config_templates/config.yml to ./scripts/ci/pre_commit/../../../airflow/providers/google/config_templates/default_config.cfg
```
But what Am I suppose to do when it compares to previous code that in fact as changed ? :
```
diff --git a/tests/security/test_kerberos.py b/tests/security/test_kerberos.py
index 2c1c92e..073d1ef 100644
--- a/tests/security/test_kerberos.py
+++ b/tests/security/test_kerberos.py
@@ -43,43 +43,43 @@ class TestKerberos(unittest.TestCase):
"""
assert renew_from_kt(principal=self.args.principal, keytab=self.args.keytab) is None
- @conf_vars({('kerberos', 'keytab'): KRB5_KTNAME, ('kerberos','include_ip'): ''})
+ @conf_vars({('kerberos', 'keytab'): KRB5_KTNAME, ('kerberos', 'include_ip'): ''})
```
I did those change to solve formatting checks and it creates more error, I don't understand.
Thank you for your help.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-932062804
Does anyone knows if by anychance this PR will be deploy on next release ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-907360622
Yep. There was GitHub Packages incident: https://www.githubstatus.com/incidents/wsh4dvm6x81k closed/reopened to rebuild.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-906365315
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-908459967
@kiwy42 - the way how pre-commit works in this case, it "fixes" things for you - but then it fails so that you can manually add those changed files and rerun commit again. Then it should work fine. Pre-commit will work by default on changes that have been staged in git (so you run git add on those changese). Most likely it modified your code but then you have not added those changes and re-run pre-commit again (which under the hood would "stash" the changes you've made and re-apply the fix again on the old version).
In short your flow with pre-commit is :
1) You run it
2) In some cases it will apply fixes automatically in others it will tell you that there are things to fix
3) You fix what you need to fix
4) You run `git add -p` (best) to apply the changes you've done and the changes done by pre-commit
5) You run commit again and observe pre-commit is happy and green
J.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-909094402
OK, now I'm over 9000 frustrated, and this error:
```
ERROR: Cannot install apache-airflow-providers-celery==2.0.0.dev0 because these package versions have conflicting dependencies.
The conflict is caused by:
apache-airflow-providers-celery 2.0.0.dev0 depends on celery>=5.1.2 and ~=5.1
The user requested (constraint) celery==4.4.7
```
Is the reason why I stopped trying to build Airflow, dependencies constraints broke so easily, I don't know how or why but this made me stop trying to build anything myself.
Hope I will one day finish this pull request :D
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-908459967
@kiwy42 - the way how pre-commit works in this case, it "fixes" things for you - but then it fails so that you can manually add those changed files and rerun commit again. Then it should work fine. Pre-commit will work by default on changes that have been staged in git (so you run git add on those changese). Most likely it modified your code but then you have not added those changes and re-run pre-commit again (which under the hood would "stash" the changes you've made and re-apply the fix again on the old version).
In short your flow with pre-commit is :
1) You run it
2) In some cases it will apply fixes automatically in others it will tell you that there are things to fix
3) You fix what you need to fix
4) You run `git add -p` (best) to apply the changes you've done and the changes done by pre-commit
5) You run commit again and observe pre-commit is happy and green
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] mik-laj commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
mik-laj commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-932070404
Most likely Airflow 2.2. As workaround, you can overwrite file `airflow/security/kerberos.py` after installation. It shouldn't be too much of a maintenance.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 edited a comment on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 edited a comment on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-908229518
> Not your fault for MSSQL (Something we need to fix) but the static check is definitely yours to fix ;).
I'm trying but those pre commit static checks I don''t understand, it compares my file with my previous file but they changed.
I'm a bit puzzled.
Also there's those fail that do not create an explicit error:
```
black....................................................................................Failed
- hook id: black
- files were modified by this hook
reformatted airflow/security/kerberos.py
reformatted tests/security/test_kerberos.py
All done! ✨ 🍰 ✨
2 files reformatted, 2725 files left unchanged.
All done! ✨ 🍰 ✨
244 files left unchanged.
```
and
```
Checks for consistency between config.yml and default_config.cfg.........................Failed
- hook id: airflow-config-yaml
- files were modified by this hook
Converting ./scripts/ci/pre_commit/../../../airflow/config_templates/config.yml to ./scripts/ci/pre_commit/../../../airflow/config_templates/default_airflow.cfg
Converting ./scripts/ci/pre_commit/../../../airflow/providers/google/config_templates/config.yml to ./scripts/ci/pre_commit/../../../airflow/providers/google/config_templates/default_config.cfg
```
But what Am I suppose to do when it compares to previous code that in fact has changed ? :
```
diff --git a/tests/security/test_kerberos.py b/tests/security/test_kerberos.py
index 2c1c92e..073d1ef 100644
--- a/tests/security/test_kerberos.py
+++ b/tests/security/test_kerberos.py
@@ -43,43 +43,43 @@ class TestKerberos(unittest.TestCase):
"""
assert renew_from_kt(principal=self.args.principal, keytab=self.args.keytab) is None
- @conf_vars({('kerberos', 'keytab'): KRB5_KTNAME, ('kerberos','include_ip'): ''})
+ @conf_vars({('kerberos', 'keytab'): KRB5_KTNAME, ('kerberos', 'include_ip'): ''})
```
I did those change to solve formatting checks and it creates more error, I don't understand.
Thank you for your help.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #17816:
URL: https://github.com/apache/airflow/pull/17816
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] eladkal commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
eladkal commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-932192703
It's already available in beta `2.2.0b2`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] kiwy42 commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
kiwy42 commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-909094402
OK, now I'm over 9000 frustrated, and this error:
```
ERROR: Cannot install apache-airflow-providers-celery==2.0.0.dev0 because these package versions have conflicting dependencies.
The conflict is caused by:
apache-airflow-providers-celery 2.0.0.dev0 depends on celery>=5.1.2 and ~=5.1
The user requested (constraint) celery==4.4.7
```
Is the reason why I stopped trying to build Airflow, dependencies constraints broke so easily, I don't know how or why but this made me stop trying to build anything myself.
Hope I will one day finish this pull request :D
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-908459967
@kiwy42 - the way how pre-commit works in this case, it "fixes" things for you - but then it fails so that you can manually add those changed files and rerun commit again. Then it should work fine. Pre-commit will work by default on changes that have been staged in git (so you run git add on those changese). Most likely it modified your code but then you have not added those changes and re-run pre-commit again (which under the hood would "stash" the changes you've made and re-apply the fix again on the old version).
In short your flow with pre-commit is :
1) You run it
2) In some cases it will apply fixes automatically in others it will tell you that there are things to fix
3) You fix what you need to fix
4) You run `git add -p` (best) to apply the changes you've done and the changes done by pre-commit
5) You run commit again and observe pre-commit is happy and green
J.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] ashb commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
ashb commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-910501457
Random failures only -- merging!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-906364500
> It's nothing too fancy but at least validate that my option do not disrupt regular operation.
Works for me!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-908459967
@kiwy42 - the way how pre-commit works in this case, it "fixes" things for you - but then it fails so that you can manually add those changed files and rerun commit again. Then it should work fine. Pre-commit will work by default on changes that have been staged in git (so you run git add on those changese). Most likely it modified your code but then you have not added those changes and re-run pre-commit again (which under the hood would "stash" the changes you've made and re-apply the fix again on the old version).
In short your flow with pre-commit is :
1) You run it
2) In some cases it will apply fixes automatically in others it will tell you that there are things to fix
3) You fix what you need to fix
4) You run `git add -p` (best) to apply the changes you've done and the changes done by pre-commit
5) You run `git commit` again and observe pre-commit is happy and green
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk edited a comment on pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #17816:
URL: https://github.com/apache/airflow/pull/17816#issuecomment-908459967
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed pull request #17816: Pull Request #17342: add support to enable kinit options [-f|-F] and [-a|-A]
Posted by GitBox <gi...@apache.org>.
potiuk closed pull request #17816:
URL: https://github.com/apache/airflow/pull/17816
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org