Re: [PR] Make default execution server URL be relative to API Base URL [airflow]
zachliu commented on PR #49747: URL: https://github.com/apache/airflow/pull/49747#issuecomment-2877125602 @potiuk Thank you for the encouragement, Professor—I was just venting. I’ll probably never stop being the first to eat the crab :joy: As for the matter at hand, the issue seems to stem from a subtle difference between `localhost` and ``. When specifying the execution API URL, it likes the ``—such as the service name `apiserver` or `webserver`—like so: `http://webserver:/execution`. Should be fine in a production environment but not for local runs when my `base_url = http://localhost:/` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Make default execution server URL be relative to API Base URL [airflow]
potiuk commented on PR #49747: URL: https://github.com/apache/airflow/pull/49747#issuecomment-2875788408 > This journey from Airflow 2 to 3 has made me reluctant to be the first to "eat the crab" again. And I hate being proven right once more… after building and installing the latest Celery provider from main, I'm still getting bombarded with ConnectError: [Errno 111] Connection refused if I don't set the AIRFLOW__CORE__EXECUTION_API_SERVER_URL environment variable somewhere. However, we greatly appreciate your efforts @zachliu -> yes, there are teething issues, and being an open-source project, it's almost a given that the first brave users who will start testing and using Airflow 3 will find things that we have not thoguht about and that's the only way we can actually get to know about those issues and fix them. There is no software with bugs, and no amount of "lab" tesitng is able to find all those - especially if you give your software for free and there is noone who will pay for that extra testing. So effectively this is the price you pay for using sotware that cost you `0 USD`. And we consider it a great contribution from people like you who are brave enough and supportive enough and understand that there might be issues - what we offer instead is a close cooperation, trying to find mitigations and offer ways to try and tests main versions and RC candidates as soon as we have them. I think - to be honest - it was great getting your initial feedback and issue reports and we will continue to diagnose and fix issues as they come. I think the power of Airlfow come from the power of the community that is both helpful and understands that there are some initial teething pains - and goes through those pains together with us. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Make default execution server URL be relative to API Base URL [airflow]
zachliu commented on PR #49747: URL: https://github.com/apache/airflow/pull/49747#issuecomment-2874825490 > Yes new provider wave is coming - but if you want you can easily (now) build provider on your own from main and test it - or even install it via github URL: > > ``` > pip install "apache-airflow-providers-celery @ git+https://github.com/apache/airflow/#subdirectory=providers/celery"; > ``` This journey from Airflow 2 to 3 has made me reluctant to be the first to "eat the crab" again. And I hate being proven right once more… after building and installing the latest Celery provider from main, I'm still getting bombarded with `ConnectError: [Errno 111] Connection refused` if I don't set the `AIRFLOW__CORE__EXECUTION_API_SERVER_URL` environment variable somewhere. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Make default execution server URL be relative to API Base URL [airflow]
potiuk commented on PR #49747: URL: https://github.com/apache/airflow/pull/49747#issuecomment-2874320501 Yes new provider wave is coming - but if you want you can easily (now) build provider on your own from main and test it - or even install it via github URL: ``` pip install "apache-airflow-providers-celery @ git+https://github.com/apache/airflow/#subdirectory=providers/celery"; ``` -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Make default execution server URL be relative to API Base URL [airflow]
zachliu commented on PR #49747: URL: https://github.com/apache/airflow/pull/49747#issuecomment-2874277614 > as that provider won't work until that Airflow version is released! should we expect new releases of the providers in a few days? for example the latest `apache-airflow-providers-celery==3.10.6 ` doesn't have the fix yet -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Make default execution server URL be relative to API Base URL [airflow]
kaxil merged PR #49747: URL: https://github.com/apache/airflow/pull/49747 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] Make default execution server URL be relative to API Base URL [airflow]
amoghrajesh commented on code in PR #49747:
URL: https://github.com/apache/airflow/pull/49747#discussion_r2059108875
##
airflow-core/src/airflow/executors/local_executor.py:
##
@@ -108,6 +108,10 @@ def _execute_work(log: logging.Logger, workload:
workloads.ExecuteTask) -> None:
from airflow.sdk.execution_time.supervisor import supervise
setproctitle(f"airflow worker -- LocalExecutor: {workload.ti.id}")
+
+base_url = conf.get("api", "base_url", fallback="/")
+default_execution_api_server = f"{base_url.rstrip('/')}/execution/"
Review Comment:
Valid point, seems fine. Yeah we will not be able to do 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Make default execution server URL be relative to API Base URL [airflow]
kaxil commented on code in PR #49747:
URL: https://github.com/apache/airflow/pull/49747#discussion_r2059105490
##
airflow-core/src/airflow/executors/local_executor.py:
##
@@ -108,6 +108,10 @@ def _execute_work(log: logging.Logger, workload:
workloads.ExecuteTask) -> None:
from airflow.sdk.execution_time.supervisor import supervise
setproctitle(f"airflow worker -- LocalExecutor: {workload.ti.id}")
+
+base_url = conf.get("api", "base_url", fallback="/")
+default_execution_api_server = f"{base_url.rstrip('/')}/execution/"
Review Comment:
The problem is that they are with separate providers. So keeping the logic
in central location has a big disadvantage where that provider won't work until
that version is released!
So the repetition is intentional
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] Make default execution server URL be relative to API Base URL [airflow]
amoghrajesh commented on code in PR #49747:
URL: https://github.com/apache/airflow/pull/49747#discussion_r2059100670
##
airflow-core/src/airflow/executors/local_executor.py:
##
@@ -108,6 +108,10 @@ def _execute_work(log: logging.Logger, workload:
workloads.ExecuteTask) -> None:
from airflow.sdk.execution_time.supervisor import supervise
setproctitle(f"airflow worker -- LocalExecutor: {workload.ti.id}")
+
+base_url = conf.get("api", "base_url", fallback="/")
+default_execution_api_server = f"{base_url.rstrip('/')}/execution/"
Review Comment:
Can we just add a property or extract this onto the base executor or
somewhere relevant?
Too many repetitions. It will be easy to miss in case of a refactor.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
