henry3260 commented on PR #73549: URL: https://github.com/apache/airflow/pull/73549#issuecomment-5779527842
> https://github.com/orgs/apache/projects/499/views/7?pane=issue&itemId=248986989 > > > * Registering after `Serve` has been called panics - registration closes when serving starts. > > * A duplicate `(dagId, taskId)` panics. > > * A struct carrying `arg:` tags mixed with other data parameters is rejected (an untagged struct has no such guard and is decoded positionally as one value). > > * Signature errors - non-function, wrong return shape, missing leading `airflow.Context` - panic with the offending function named. > > It seems the scope of current PR is not align with the issue scope. How about checking whether all the above issue scope are covered instead of having current refactor. > > Additionally, the task id validation should be in compile time instead of at the task runtime. IIUC, the current refactor will introduce side-effect (might raise validation error) during the runtime. Thanks I had misunderstood earlier. Implemented the first point instead: registration now closes in `Serve`, and a later `Register` panics. It was the only one of the four `main` did not already satisfy. I kept the variadic check as a signature error of the same class: `Execute`calls the handler with `Call`, which cannot spread into a `...` parameter, so it registers cleanly and panics inside reflect on first run. It sits in `validateFn` beside the existing checks. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
