potiuk edited a comment on pull request #20338:
URL: https://github.com/apache/airflow/pull/20338#issuecomment-995866427
As discussed today, good start. One thing - it shoudl rather be quite
different in terms of "global" variables. It's better to have a common way to
build a structure, where we pass all the needed parameters (for example
BuildParams: TypedDict) that will get all the parameters needed to execute the
build and get a common logic to build this "BuildParams" object from the
options provided.
A lot of this commmon variables in Bash was that there is not an easy and
straightforward way to execute a common code in Bash, and calculating global
env variables once in common code seemed like the most efficient way of doing
it, but this does not have to be done this way here.
I think BuildParams should be constructed to pass all the raw-input
parameters needed to derive the final parameters, but then all the actual
values passed can be calculated on the flight by some methods that take those
params.
For example this:
```
function build_images::get_github_container_registry_image_prefix() {
echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]'
}
local image_name
image_name="ghcr.io/$(build_images::get_github_container_registry_image_prefix)"
# Example:
# ghcr.io/apache/airflow/main/ci/python3.8
export
AIRFLOW_CI_IMAGE="${image_name}/${BRANCH_NAME}/ci/python${PYTHON_MAJOR_MINOR_VERSION}"
# Example:
# ghcr.io/apache/airflow/main/ci/python3.8:latest
# ghcr.io/apache/airflow/main/ci/python3.8:<COMMIT_SHA>
export
AIRFLOW_CI_IMAGE_WITH_TAG="${image_name}/${BRANCH_NAME}/ci/python${PYTHON_MAJOR_MINOR_VERSION}:${GITHUB_REGISTRY_PULL_IMAGE_TAG}"
```
Could become this in Python (typing from memory so it might not compile :) :
```python
class BuildParams(TypedDict):
pythonVersion: str
githubRepository: str
branchName: str
....
def get_airflow_ci_image(buildParams: BuildParams, tag: Optional[str] =
None):
image=f'ghcr.io/{buildParams.gitubRepository.lower()}/{buildParams.branchName}/ci/python{buildParams.pythonVersion}'
return image if not tag else image + f":{tag}"
```
or even this (this one is way better:
```python
@dataclass
class BuildParams:
pythonVersion: str
githubRepository: str
branchName: str
....
def get_airflow_ci_image(self, tag: Optional[str] = None):
image=f'ghcr.io/{self.gitubRepository.lower()}/{sekf.branchName}/ci/python{self.pythonVersion}'
return image if not tag else image + f":{tag}"
```
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
