https://github.com/stitchfix/hamilton logo
Join Slack
Powered by
<@U03AV0GG20H> What are your thoughts on adding a ...
# contribute
i
@Stefan Krawczyk What are your thoughts on adding a hamilton provider to airflow? I don't have a clear view of what it should include yet, but for start - we can have an operator (HamiltonOperator) that sets up a builder and produces a file on disk or a serializable object
s
Hamilton just needs python dependencies to work
i
Perhaps my question wasn't clear - the idea is to create a plugin for airflow that would make it easier to invoke hamilton
s
Yep. Sorry. Inputs would be modules and outputs to compute...
i
And configs
s
Right
And plugins for whatever serde stuff airflow is doing between jobs?
Or tasks?
i
Right - could be a custom XCom backend. Which brings me to the main issue I currently see, which is with dealing with outputs, since Airflow best practices state that they should ideally be primitive and minimal (a specific example: instead of outputting dataframes, the task should save it to disk and produce a path to a file)
I can think about this some more and come up with a more thorough proposal, then perhaps request some feedback from Jarek. Just thought to check with you first if you think this might be useful
👍 1
@Stefan Krawczyk -1/0/+1?
I mainly want feedback on: • Whether the split between Operator and decorator, the way it was done, makes sense • Did I miss any common use cases? • The scope of v1 vs later versions
Also - can you please reserve the community package name on pypi?
j
You will have to start discussion on devlist anyway (Though we are likely to relax it a bit) - there is an ongoing discussion
reserved it
gratitude thank you 1
Genrally cool, but there are a few things that's worth considering before attempting to make PR • we are about to drop Airflow 2 support from providers - so likely (and better for you) dropping it and being 3-only is the only path • which means you have to drop the direct DB access from it (which I believe you still have) • save_to rather in relation to airflow custom Xcom_backend should be rethought - it's not true any more that Xcoms should be small - this is only if you do not configure proper backends and with common.io backends you can even automatically route XCom to DB or external storage depending on size • not sure if HamiltonDataClass for decorator is a good idea - airflow task generally prefer individual values and only those that are needed - dataclass is not too airflow-y approach - and it does not fully integrate with airflow, especially for mapped tasks • lack of OpenLineage integration is a big minus • I think there is something wrong with the MRO hierarchy for async/sync - it's not how we do it in Airflow I think - I think also deferrable/async support might be broken by sync driver execute (but It was mostly a glance) • input and config are templateable but also json - which means it wont' work unless you specify native template rendering for the Dag • tests and docs are mandatory
👍 1
Those are my comments (Claude was used to help with those - but I reviewed it and wrote it with my own understanding)
i
Proposal updated with the above feedback. Dziękuję!
j
He He . Did you use YOUR agent to apply it 🙂 ?
I can't imagine doing it by hand
i
of course I did
I wasn't entirely lazy and supplied it with several documentation links to take inspiration from
👍 1
j
Same. Lazy - but not entirely 😄
i
> You will have to start discussion on devlist anyway Do you mean discuss down the road (in the airflow devlist) when we want to integrate this into the airflow repo (
apache-airflow-providers-apache-hamilton
), or even now, if this is implemented as a submodule in the hamilton repo (
airflow-provider-hamilton
)? Are you suggesting to implement it directly in airflow?
Just added the Integration Tests section
2 Views
Open in Slack
PreviousNext
Powered by