chore: add AthenaConfig to impl.py

[SoumayaMauthoorMOJ]

Description

Add AthenaConfig to impl.py following recommendations

I have added the Table configuration as well as the connection parameters that can be passed to cursor.execute()

This might help to resolve #377, but it doesn't fix it by itself

Models used to test - Optional

I ran the unit tests locally and they all passed
I ran some of the functional tests and they passed too, but I didn't run all of them

Checklist

• You followed contributing section
• You kept your Pull Request small and focused on a single feature or bug fix.
• You added unit testing when necessary
• You added functional testing when necessary

[nicor88]

I believe that there is still some work to do no?
In #377 we decided to pick the workgroup from model config and pass it down to adapter.run_query_with_partitions_limit_catching, this means that in each macro you need to:

• pick the workgroup from config using jijia-> {%- set work_group = config.get('work_group', default=target.work_group) -%}
• inject workgroup when possible when queries are executed

[SoumayaMauthoorMOJ]

Hey @nicor88 you're right there is still some work to do, but I thought I would start off with a smaller change first which might help with #377. In the meantime I've raised a question on slack channel https://getdbt.slack.com/archives/CBSQTAPLG/p1694451069966319 about the best way of tackling this. I'm worried about the amount of refactoring required if I modify all the materialisations.

[nicor88]

@SoumayaMauthoorMOJ let's see if we get some nice suggestions, maybe you can even consider to ask in db-athena slack channel to gather mode athena experts :)

[SoumayaMauthoorMOJ]

@nicor88 Did not know there was a slack channel raising now :-)

[svdimchenko]

the code looks nice, let's add missed parameters and docstring and we are ready to go

[SoumayaMauthoorMOJ]

@svdimchenko I don't have permission to add the label to run the functional tests, do think it's worthwhile running those?

Also not too sure what docstring to use because explanation on dbt website isn't very clear: "database- and relation-level configs"

Hence why I raised a question on slack. I'll get the attribute descriptions from the README.

[svdimchenko]

@svdimchenko I don't have permission to add the label to run the functional tests, do think it's worthwhile running those?

Also not too sure what docstring to use because explanation on dbt website isn't very clear: "database- and relation-level configs"

Hence why I raised a question on slack. I'll get the attribute descriptions from the README.

I agree with you, docstring is not obligatory. However we should have all parameters defined I guess. So this comment should be fixed: #412 (comment)

[svdimchenko]

@SoumayaMauthoorMOJ pls install and run pre-commit locally to avoid pre-commit checks fail in PR. Once it's fixed we are ready to go

[nicor88]

@SoumayaMauthoorMOJ it's recommended for new contributors to follow the contributing section.
In there there is a step called make setup that also takes care of installing the pre-commit hooks, therefore the same checks that we execute in the CI are executed before commiting - please have a look - exactly as @svdimchenko suggested btw.

[SoumayaMauthoorMOJ]

Sorry will do this evening :-)