Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-14 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2055439942 Thank you @potiuk :) -- 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.

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-14 Thread via GitHub
potiuk merged PR #38111: URL: https://github.com/apache/airflow/pull/38111 -- 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:

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-14 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2054164536 The docker example error workarounded in main. 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

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-12 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2052628759 some main errrs fixed already - you will need to rebase -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-12 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2051971605 > Those are new tests you addded that are now failing, It's a different error you get: > > You also need to mark the test as db_test per instruction: > > > FAILED

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-12 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2051965928 Those are new tests you addded that are now failing, It's a different error you get: You also need to mark the test as db_test per instruction: > FAILED

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-12 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2051846148 > See how it it's done in main (and you rebased to it) `pytestmark` is assigned at the whole module level`- so all those tests are now`db_tests` - your previous attempt removed all of

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-12 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2051805887 See how it it's done in main (and you rebased to it) `pytestmark` is assigned at the whole module level` - so all those tests are now `db_tests` - your previous attempt removed all of

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-12 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2051782973 > You should remove your fix and rebase - in change you applied you removed the markers on tests but it needs `pytestmark` on the module level instead, So I need to do like this

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-12 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2051667128 You should remove your fix and rebase - in change you applied you removed the markers on tests but it needs `pytestmark` on the module level instead, -- This is an automated message

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-12 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2051052690 I will just add an example for the MSGrapSensor and how we use it with the PowerBI api -- This is an automated message from the Apache Git Service. To respond to the message, please

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050548933 > I told you adding local imports is **really** not a good idea - those imports are needed elsewhere (in unit tests where they are mocked). Yeah saw it too, moved them at top of

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050548094 > Surely, but we have the same problem with common code. If we add it today, it's only going to be really usable 10 months from now. because providers are importing stuff from earlier

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050393302 I told you adding local imports is **really** not a good idea - those imports are needed elsewhere (in unit tests where they are mocked). -- This is an automated message from the

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050266922 @potiuk FYI: I've also deleted the apache-airflow-providers-msgraph package under pypi -- This is an automated message from the Apache Git Service. To respond to the message, please

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050247170 > > But if you confirm this is fine then no problem I will do it like this :) > > Yes. We do it in a number of other places - you already looked at it. > > > couldn't we

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050167395 > But if you confirm this is fine then no problem I will do it like this :) Yes. We do it in a number of other places - you already looked at it. > couldn't we adapt the

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050135785 > Some more explanation: This is what compatiblity checks are doing basically - they try to import the providers in airflow 2.6.0 so if you import anything that is missing, they should

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050133314 > > Now I'm getting this [errors](https://github.com/apache/airflow/actions/runs/8648788861/job/23713776144?pr=38111#step:10:2621) probably something else needs to be changed/excluded so

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050116244 Some more explanation: This is what compatiblity checks are doing basically - they try to import the providers in airflow 2.6.0 so if you import anything that is missing, they should

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2050112072 > Now I'm getting this [errors](https://github.com/apache/airflow/actions/runs/8648788861/job/23713776144?pr=38111#step:10:2621) probably something else needs to be changed/excluded so

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2049944633 > "microsoft-azure" -> "microsoft.azure" : this is a short id we are using for providers - with `.` -> see "common.io". Likely PR where we normalize it would be good. Now I'm

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2049873688 > "microsoft-azure" -> "microsoft.azure" : this is a short id we are using for providers - with `.` -> see "common.io". Likely PR where we normalize it would be good.

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2049820196 https://github.com/apache/airflow/pull/38936 -- 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

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2049801878 "microsoft-azure" -> "microsoft.azure" : this is a short id we are using for providers - with `.` -> see "common.io". Likely PR where we normalize it would be good. -- This is an

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2049780531 > > it can become a dependency hell > > BTW. Yes. It's a bit contained hell and the rules we have about min-airflow version and separting providers out is our way

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2049444186 > > it can become a dependency hell > > BTW. Yes. It's a bit contained hell and the rules we have about min-airflow version and separting providers out is our way of dealing with

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2049423633 > it can become a dependency hell BTW. Yes. It's a bit contained hell and the rules we have about min-airflow version and separting providers out is our way of dealing with it.

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2049370288 > Alright, thank you Jarek, indeed it can become a dependency hell when you have to deal with so much dependencies. Will update this in the provider.yaml I was trying to reproduce it

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2049360149 > Hey @dabla > > I think you should update `apache-airflow>=2.7.0` in mysql-azure provider and add `mysql-azure` to the

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-11 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2049355078 Hey @dabla I think you should add `apache-airflow>=2.7.0` in mysql provider and add mysql to the

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-04-05 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2040760143 I ma afraid conflicts need to be resolved with rebase. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-03-27 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2024146526 > > You should **just** rebase. "Always Be Rebased" as the old Chinese proverb says. > > Who would be kind enough to review this PR? I just can't figure out what's wrong the the

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-03-27 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2022384426 > You should **just** rebase. "Always Be Rebased" as the old Chinese proverb says. Who would be kind enough to review this PR? I just can't figure out what's wrong the the docs

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-03-20 Thread via GitHub
potiuk commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-2009209802 You should **just** rebase. "Always Be Rebased" as the old Chinese proverb says. -- This is an automated message from the Apache Git Service. To respond to the message, please log on

Re: [PR] Adding MSGraphOperator in Microsoft Azure provider [airflow]

2024-03-15 Thread via GitHub
dabla commented on PR #38111: URL: https://github.com/apache/airflow/pull/38111#issuecomment-1999330233 Why are there suddenly tests failing of classes I didn't even touch? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and