You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "maiconkkl (via GitHub)" <gi...@apache.org> on 2023/12/26 14:40:49 UTC
[PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
maiconkkl opened a new pull request, #36437:
URL: https://github.com/apache/airflow/pull/36437
related: #36432
The exception raised Your CUSTOM_SECURITY_MANAGER must extend FabAirflowSecurityManagerOverride, not FAB's own security manager.
My English is not good, so I ask you to be patient.
The error happens when extending the FabAirflow SecurityManagerOverride class to create custom authentication
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1874395446
@maiconkkl if you could pull the changes of this PR locally and do some testing on your end, that would be very appreciated :)
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1874394749
See #36538
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1869649485
I think the intention here was a bit different and I'd wait for @vincbeck to come back from holidays to merge this one. There were a few changes to the structure of the Auth Manager class and @vincbeck has PRs with doc in progress to describe them better - so I'd keep it around till he is back.
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "maiconkkl (via GitHub)" <gi...@apache.org>.
maiconkkl commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1874419943
> @maiconkkl if you could pull the changes of this PR locally and do some testing on your end, that would be very appreciated :)
I already tested it and it works well 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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "DjVinnii (via GitHub)" <gi...@apache.org>.
DjVinnii commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1870432714
I'm also running into the related issue, so a fix will be great as it is now blocking us to upgrade to Airflow 2.8. To me the proposed fix should do the trick as the raised error indicates the check is inverted.
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1869656502
Not really. We are cherry-picking some changes from main to be released in 2.8.* - these are bug-fixes and doc updates mostly - so.I'd say this (and doc changes) should go to 2.8.1. it is about more complicated because in main we already moved FAB to separate provider, which will make cherry-picking a bit more complex - that's why as well I think we should do any fixes here while @vincbeck is around to make sure we all make sure we are doing the right things.
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "maiconkkl (via GitHub)" <gi...@apache.org>.
maiconkkl closed pull request #36437: Fix Exception FabAirflowSecurityManagerOverride
URL: https://github.com/apache/airflow/pull/36437
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "boring-cyborg[bot] (via GitHub)" <gi...@apache.org>.
boring-cyborg[bot] commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1869588628
Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:
- Pay attention to the quality of your code (ruff, mypy and type annotations). Our [pre-commits]( https://github.com/apache/airflow/blob/main/STATIC_CODE_CHECKS.rst#prerequisites-for-pre-commit-hooks) will help you with that.
- In case of a new feature add useful documentation (in docstrings or in `docs/` directory). Adding a new operator? Check this short [guide](https://github.com/apache/airflow/blob/main/docs/apache-airflow/howto/custom-operator.rst) Consider adding an example DAG that shows how users should use it.
- Consider using [Breeze environment](https://github.com/apache/airflow/blob/main/BREEZE.rst) for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
- Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
- Please follow [ASF Code of Conduct](https://www.apache.org/foundation/policies/conduct) for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
- Be sure to read the [Airflow Coding style]( https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#coding-style-and-best-practices).
- Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
Apache Airflow is a community-driven project and together we are making it better 🚀.
In case of doubts contact the developers at:
Mailing List: dev@airflow.apache.org
Slack: https://s.apache.org/airflow-slack
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "maiconkkl (via GitHub)" <gi...@apache.org>.
maiconkkl commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1874364103
> I am against this change.
>
> What is the value of `SECURITY_MANAGER_CLASS` in your scenario? There is something weird because this check was already there before we migrated the code to fab provider, so it should not be a breaking change. We want to make sure whatever class you defined in `SECURITY_MANAGER_CLASS` inherits from `AirflowSecurityManager`. Again, this check was there before so there must be something off somewhere.
Hello vincbeck, how are you? Happy New Year!!!
When trying to extend AirflowSecurityManager I receive a DeprecationWarning with the following message
"Call to deprecated class AirflowSecurityManager. (If you want to override the security manager, you should inherit from `airflow.auth.managers.fab.security_manager.override.FabAirflowSecurityManagerOverride` instead)"
And when trying to extend FabAirflowSecurityManagerOverride I receive an Exception with the following message
"Your CUSTOM_SECURITY_MANAGER must extend FabAirflowSecurityManagerOverride, not FAB's own security manager."
Interpreting what the warnings say, I understand that the AirflowSecurityManager class is becoming obsolete and needs to extend the FabAirflowSecurityManagerOverride class and my change proposes exactly that
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1874385253
Happy new year too @maiconkkl. Thanks for all the details and indeed there is a bug. This is related to #36343 as well. I just figured out the bug, let me create a PR for it. I'll explain the details there
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1874423322
> > @maiconkkl if you could pull the changes of this PR locally and do some testing on your end, that would be very appreciated :)
>
> I already tested it and it works well for me
Thank you!
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1870450698
> I'm also running into the related issue, so a fix will be great as it is now blocking us to upgrade to Airflow 2.8. To me the proposed fix should do the trick as the raised error indicates the check is inverted.
It will stil have to be released in 2.8.1 and it will take some time. So feel free to apply the patch for now in your version - this is the fastest way you can get it fixed for you now.
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1870451711
This is a beauty of Open-Source software. You can ALWAYS patch things while waiting for a more complete ifx. And rushing fixes without having a full knowledge is often bad idea. Go slow to move fast.
--
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
Re: [PR] Fix Exception FabAirflowSecurityManagerOverride [airflow]
Posted by "maiconkkl (via GitHub)" <gi...@apache.org>.
maiconkkl commented on PR #36437:
URL: https://github.com/apache/airflow/pull/36437#issuecomment-1869652338
> I think the intention here was a bit different and I'd wait for @vincbeck to come back from holidays to merge this one. There were a few changes to the structure of the Auth Manager class and @vincbeck has PRs with doc in progress to describe them better - so I'd keep it around till he is back.
I haven't seen these PRs, however they are intended for 2.8.* ?
I saw that there were some changes in main, but I believe it would be for a new version, maybe 2.9
--
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