ginone commented on PR #45634: URL: https://github.com/apache/airflow/pull/45634#issuecomment-2619507526
> @josh-fell @Lee-W thank you for the code review, I've addressed all feedback. Could you please look again? > > BTW. What do you think regarding @jaklan suggestion ([#45634 (review)](https://github.com/apache/airflow/pull/45634#pullrequestreview-2557110660))? My thoughts: > > 1. this (implicit argument types) feels wrong/confusing: > > ```python > self.job: str | int, > self.project: str | int, > self.environment: str | int, > ``` > > I think the params should be explicit, so I would go with: > > ```python > self.job_id: int, > self.project_id: int, > self.environment_id: int, > self.job_name: str, > self.project_name: str, > self.environment_name: str, > ``` > > 2. I see this as an enhancement that could allow more flexible setups, but I would prefer to release this PR "as is" and unblock using `project_name`, `environment_name`, and `job_name` ASAP. I would create a new PR with the suggested enhancements. New PR that includes the changes described above: https://github.com/apache/airflow/pull/46184 Feel free to select just one of my PRs and close the other one. -- 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
