Re: [PR] Replace `principle` by `principal` in kerberos-related code [airflow]
eladkal merged PR #43679: URL: https://github.com/apache/airflow/pull/43679 -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Replace `principle` by `principal` in kerberos-related code [airflow]
eladkal commented on PR #43679: URL: https://github.com/apache/airflow/pull/43679#issuecomment-2483471977 Lets add `43679.significant.rst` in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments). Just need to say in the file that we renamed the function due to misspelling. This will be used to generate the release notes. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Replace `principle` by `principal` in kerberos-related code [airflow]
brouberol commented on PR #43679: URL: https://github.com/apache/airflow/pull/43679#issuecomment-2483443671 I've reworked this patch to only rename the affected function, and have opened https://github.com/apache/airflow/pull/44150 that attempts to import the fixed function and falls back to the typoed one, as suggested in https://github.com/apache/airflow/pull/43679#issuecomment-2460343939 -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Replace `principle` by `principal` in kerberos-related code [airflow]
eladkal commented on PR #43679: URL: https://github.com/apache/airflow/pull/43679#issuecomment-2460343939 Cool but actually we need (2) ready before (1) All you need to do here is to remove the changes to `airflow/security/kerberos.py` and `tests/security/test_kerberos.py`. Keep the changes you have in providers but change the imports to: ``` try: from airflow.security.kerberos import get_kerberos_principal except ModuleNotFoundError: from airflow.security.kerberos import get_kerberos_principle ``` Once you do that provider is compatible with both name and I can do the change to airflow core. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Replace `principle` by `principal` in kerberos-related code [airflow]
brouberol commented on PR #43679: URL: https://github.com/apache/airflow/pull/43679#issuecomment-2460078421 I can give it a go, but I'll be a bit tied up in the coming days. Feel free to proceed with 1) if you want. If not, I'll try to address your comment when I have the time. Thanks again! -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Replace `principle` by `principal` in kerberos-related code [airflow]
eladkal commented on PR #43679: URL: https://github.com/apache/airflow/pull/43679#issuecomment-2458946460 OK lets take it by steps: 1. changes to `airflow/security/kerberos.py` should be in a separated PR. Add new function `get_kerberos_principal` with the logic and deprecate `get_kerberos_principle` once this PR is merged to main branch, we can cherry pick it to v2.10-test branch. 2. once (1) is completed we can handle the provider changes -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Replace `principle` by `principal` in kerberos-related code [airflow]
potiuk commented on PR #43679: URL: https://github.com/apache/airflow/pull/43679#issuecomment-2458272499 > Still I think we can do it with Airflow 3 and backport to 2.10.. this is pretty minor and easy to do. If @brouberol wishes to do the back-compatiblity code for both issues, we can do it, yes. -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Replace `principle` by `principal` in kerberos-related code [airflow]
eladkal commented on PR #43679: URL: https://github.com/apache/airflow/pull/43679#issuecomment-2456974285 > I think the hassle is not worth the typo fix We can just have a function with the correct name and deprecate the old one given that we are going to remove all deprecations before 2.11 I think it's good chance to do it -- 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-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
