Skip to content

Conversation

@bdsoha
Copy link
Contributor

@bdsoha bdsoha commented Jun 23, 2025

The EdgeExecutor defaults to localhost:8080 for core.execution_api_server_url, which does not immediately fail, but causes errors once a job is dequeued.
The newly added documentation guides users on how to properly configure core.execution_api_server_url.

Alternatively, the implementation logic itself could be improved by deriving the value from edge.api_url instead.


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in airflow-core/newsfragments.

@boring-cyborg boring-cyborg bot added area:providers kind:documentation provider:edge Edge Executor / Worker (AIP-69) / edge3 labels Jun 23, 2025
@bdsoha bdsoha marked this pull request as ready for review June 23, 2025 10:15
@bdsoha bdsoha requested a review from jscheffl as a code owner June 23, 2025 10:15
@bdsoha
Copy link
Contributor Author

bdsoha commented Jun 23, 2025

@jscheffl If you agree with the alternative implementation I described above, I would be happy to open another PR for it.
What do you think?

Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for documenting. I never came across this - problably as I always configured correctly.

Other than docs we could also force to set it "better" for example in https://github.com/apache/airflow/blob/main/providers/edge3/src/airflow/providers/edge3/cli/edge_command.py#L81 so that if this is not properly set that we "guess" and assume is is the same base like the edge API URL (which is the same base in 99% of cases...)

@jscheffl jscheffl merged commit b7e67d6 into apache:main Jun 23, 2025
82 of 85 checks passed
@bdsoha bdsoha deleted the feature/edge-base-url branch June 23, 2025 21:36
@bdsoha
Copy link
Contributor Author

bdsoha commented Jun 23, 2025

Other than docs we could also force to set it "better" for example in https://github.com/apache/airflow/blob/main/providers/edge3/src/airflow/providers/edge3/cli/edge_command.py#L81 so that if this is not properly set that we "guess" and assume is is the same base like the edge API URL (which is the same base in 99% of cases...)

That is what I mentioned here, should I open a PR for that?

@jscheffl
Copy link
Contributor

Other than docs we could also force to set it "better" for example in https://github.com/apache/airflow/blob/main/providers/edge3/src/airflow/providers/edge3/cli/edge_command.py#L81 so that if this is not properly set that we "guess" and assume is is the same base like the edge API URL (which is the same base in 99% of cases...)

That is what I mentioned here, should I open a PR for that?

If you like that would be cool!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers kind:documentation provider:edge Edge Executor / Worker (AIP-69) / edge3

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants