search

hoverinc / tableau-utilities

A module and CLI Utility for managing Tableau objects, locally, and in Tableau Online.
MIT License
4 stars 1 forks source link

rough start #82

Closed jaybythebay closed 6 months ago

jaybythebay commented 6 months ago

@JustinGrilli I find the Tableau Utilities DAG a little hard to read because of it's length. What do you think about pushing functionality to an optional subpackage now that we know how to configure that.

Then we could have airflow pip install tableau_utilities[airflow] and the DAG would be very simple.

If that sounds ok to you I can take the lead on the refactor. Not sure if I have time immediately this week but let me know if this looks good to as an approach.

JustinGrilli commented 6 months ago

I don't want to be responsible for other's Airflow DAGs haha. If we make the Airflow sample an actual part of the package, then we'd be responsible for ensuring the Airflow code remains functional and can be used more generally. I don't mind occasionally updating the sample as we continue to iterate on our own DAG, but there may be things we do to our DAG that will not work for others.

That being said, if you want to make the sample code more modular, go for it!

jaybythebay commented 6 months ago

@JustinGrilli I hear you and your concerns make sense. I know I've mentioned this before so thanks for humoring me in bringing it up again.

I'll figure something out over time. We could move the samples to multiple files but that's it's own headache for adding the sample here.

We could also move some functionality to the main package like "diff config and a column" in a generalized sense. That would also slim things down in the DAG and splits the difference with your concerns.

I'll close this out and think about it over time and submit PRs later.

Thanks!