search

MeltanoLabs / target-athena

Singer.io Target for AWS Athena.
Other
5 stars 16 forks source link

Add Athena Workgroup config #39

Closed ndrluis closed 1 year ago

giamo commented 2 years ago

Any chance this will be merged? It's useful when working with multiple workgroups

tayloramurphy commented 1 year ago

@pnadolny13 @edgarrmondragon thoughts about getting this across the finish line?

edgarrmondragon commented 1 year ago

@pnadolny13 @edgarrmondragon thoughts about getting this across the finish line?

@tayloramurphy Seems easy enough to apply AJ's suggestions, though I don't know if we have an Athena deployment with which to test the changes

pnadolny13 commented 1 year ago

@edgarrmondragon @tayloramurphy I was able to test this on a personal athena database and it worked as expected. I actually dont think we need to apply any changes, AJ made some minor suggestions that sound like they decided against. Accessing the config by coalesce the environment variables is status quo right now for all the other configs so I actually think its fine to add this one with the same behavior. Resolving the poetry.lock conflicts seems like the only thing required right now but I'm not exactly sure what the best way is to do that on a fork branch.

Also while trying to test on main as a base case before testing this branch, it turns out I was getting Required argument "s3_staging_dir" or "work_group" not found. because s3_staging_dir is an available config option that isnt advertised. When I switched to the working group branch it was able to use the default primary working group and succeed, so this makes that bug less relevant. I created https://github.com/MeltanoLabs/target-athena/issues/44 to track that.