Improved Armada Airflow Operator #3672
Conversation
| @@ -46,11 +46,8 @@ jobs: | |||
| runs-on: ubuntu-22.04 | |||
| strategy: | |||
| matrix: | |||
| python: [ '3.8', '3.9', '3.10' ] | |||
| python: [ '3.10' ] | |||
There was a problem hiding this comment.
Do we want to support any python versions other than 3.10?
magefiles/main.go
Outdated
| @@ -218,7 +218,7 @@ func LocalDev(arg string) error { | |||
|
|
|||
| switch arg { | |||
| case "minimal": | |||
| os.Setenv("ARMADA_COMPONENTS", "executor,server,scheduler") | |||
| os.Setenv("ARMADA_COMPONENTS", "executor,server,scheduler,lookoutingesterv2") | |||
There was a problem hiding this comment.
do we need this? I think in the docker-compose the server depend on the lookoutingester. If it doesn't , then it should!
scripts/build-airflow-operator.sh
Outdated
| @@ -1,18 +1,2 @@ | |||
| #!/bin/bash | |||
| # This script is intended to be run under the docker container at $ARMADADIR/build/python-api-client/ | |||
There was a problem hiding this comment.
presumably we can just delete this file?
third_party/airflow/README.md
Outdated
| start_date=datetime(2022, 1, 1), | ||
| catchup=False, | ||
| user_defined_macros={ | ||
| 'version': vid |
third_party/airflow/README.md
Outdated
| securityContext=core_v1.SecurityContext(runAsUser=1000), | ||
| resources=core_v1.ResourceRequirements( | ||
| requests={ | ||
| "cpu": api_resource.Quantity(string="120m"), |
There was a problem hiding this comment.
why do we have these weird quantities!
| @@ -0,0 +1,252 @@ | |||
| # Copyright 2016-2024 The Apache Software Foundation | |||
There was a problem hiding this comment.
We should add this license to all files or none of them. @dave-gantenbein?
There was a problem hiding this comment.
Not to all files, that would be overly cumbersome!
There was a problem hiding this comment.
in which case can we get rid of the license header from these files
| import grpc | ||
|
|
||
|
|
||
| class GrpcChannelArgs: |
There was a problem hiding this comment.
I think we should add documentation here as it's not obvious what all these parameters are.
There was a problem hiding this comment.
we should also explain why this class exists; i.e. because we can't serialise a grpc channel.
| lookout_url_template: Optional[str] = None, | ||
| poll_interval: int = 30, | ||
| container_logs: Optional[str] = None, | ||
| token_retriever: Optional[TokenRetriever] = None, |
There was a problem hiding this comment.
we don't have doc for the token retriever (should it be called k8s_token_retriever?).
…itHub/new-airflow-operator
Fixes #
Special notes for your reviewer:
Summary of changes:
lookoutIngesterv2to be run as part of themage localDev minimalcommand. This is required for the Query API.python3.10and above for the Armada Airflow Operator