Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate Edge calls for Worker to FastAPI part 4 - Cleanup #44434

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

jscheffl
Copy link
Contributor

After PR #44311, #44330 and #44433 this PR is a cleanup and removes the old Internal API extensions.

For review: Only the last commit is relevant, the other 9 are from referenced base PRs

@jscheffl jscheffl added area:providers AIP-69 Edge Executor provider:edge Edge Executor / Worker (AIP-69) labels Nov 27, 2024
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 11 out of 26 changed files in this pull request and generated 1 suggestion.

Files not reviewed (15)
  • docs/spelling_wordlist.txt: Language not supported
  • providers/src/airflow/providers/edge/CHANGELOG.rst: Language not supported
  • providers/src/airflow/providers/edge/worker_api/routes/_v2_routes.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/worker_api/auth.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/worker_api/datamodels.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/worker_api/app.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/worker_api/routes/rpc_api.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/worker_api/routes/logs.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/worker_api/routes/jobs.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/init.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/worker_api/routes/_v2_compat.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/executors/edge_executor.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/models/edge_logs.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/models/edge_job.py: Evaluated as low risk
  • providers/src/airflow/providers/edge/cli/edge_command.py: Evaluated as low risk

return datetime.fromisoformat(result)


def worker_set_state(
Copy link
Preview

Copilot AI Nov 27, 2024

Choose a reason for hiding this comment

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

The docstring for the worker_set_state function is misleading. It should be updated to reflect that this function updates the worker state, not registers it.

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AIP-69 Edge Executor area:providers kind:documentation provider:edge Edge Executor / Worker (AIP-69)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant