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

Refactor for 2.0.0 #9

Closed JustinGrilli closed 1 year ago

JustinGrilli commented 2 years ago

THIS IS A DRAFT - WORK IN PROGRESS

Summary

I wanted to re-imagine the tableau-utilities package.

I want to add much more functionality to the TableauServer class - and have the outputs be clearly defined as dataclass objects.

I want to remove the concept of tds from the end user - they should just think of the file as a Datasource. They should not need to concern themselves with extracting and repackaging a tdsx file - to them, they are just making updates, and saving those changes.

The TDS class would now be Datasource. And in the future, we can add Workbook. Rather than tds.add('column', a='bunch', of='args') you should now be able to datasource.columns.add(Column). This way all of the attributes are clearly defined in dataclasses, for the objects we would like to add/update/delete.

Changes

Development Changes

Notes to reviewer

Since this is a major refactor/change, I think it is worthy of being considered a major release to 2.0.0.

Tests

jaybythebay commented 2 years ago

@JustinGrilli I'm in hawaii now and just taking a peek at this before everyone wakes up. I'm diving into all the Tableau stuff further when I'm back. Here's a few questions about the PR:

  1. Do you have other things you wanted to do or check before merging this? if so, how can I help.
  2. Does this impact the Airflow code? My plan is to refactor that to make it more modular and easier to read if nothing else. This looks like it would be mostly different but just curious on that.
JustinGrilli commented 2 years ago

@JustinGrilli I'm in hawaii now and just taking a peek at this before everyone wakes up. I'm diving into all the Tableau stuff further when I'm back. Here's a few questions about the PR:

  1. Do you have other things you wanted to do or check before merging this? if so, how can I help.
  2. Does this impact the Airflow code? My plan is to refactor that to make it more modular and easier to read if nothing else. This looks like it would be mostly different but just curious on that.
  1. Yes, this is very much WIP; it's still a draft PR because there is much testing and updating left to be done. a. I need to refresh myself on this code, because it has been a weekend hobby for me, and I've been working on other things in my free time recently - but definitely would love to pair on this if you are interested!
  2. This will 100% impact the Airflow code, it will be a breaking change release - hence why I'm intending this to be a 2.0.0 release. a. There are still lots of design and functionality things I want to think through, with the tableau_file Datasource class especially. I would love to brainstorm this with you as well at some point if you are interested 😄
jaybythebay commented 2 years ago

@JustinGrilli yeah it would be great to brainstrom and coordinate on this. What does Friday the 23rd look like for you? I'm wide open.

jaybythebay commented 1 year ago

@JustinGrilli I can't seem to pull this branch. Is there some setting that is blocking me?

When I run list branches I don't see this one:


jayrosenthal@US-L379 tableau_utilities % git ls-remote origin
21b453d4c7b7762371a65eac1072e63b3c6668da        HEAD
d1b3ef3b63b44b6ca161b99fd12737577c4504f5        refs/heads/fix-airflow-example
2697bd797c51a5a27b36354a564a27d1a7208a8a        refs/heads/generate-config-from-datasource
741211767e9bc2970a441dc1f8f8ee077cc263e7        refs/heads/justin-chat
21b453d4c7b7762371a65eac1072e63b3c6668da        refs/heads/main
``

Can you check the repo settings to see if there's a privacy restriction or merge this into main?  I want to use this version of the code if possible to help test your work.  I was going to create a branch off this if possible
jaybythebay commented 1 year ago

@JustinGrilli Nevermind, I figured it out. i had forked the repository and was looking at my fork. I'm good to go now.