apache / airflow

Apache Airflow - A platform to programmatically author, schedule, and monitor workflows
https://airflow.apache.org/
Apache License 2.0
37.13k stars 14.31k forks source link

ODBC Hook documention doesn't say how to provide a connection ID #25462

Open isaacnorman82 opened 2 years ago

isaacnorman82 commented 2 years ago

What do you see as an issue?

In this document (which may have moved since the stable release) https://airflow.apache.org/docs/apache-airflow-providers-odbc/stable/_api/airflow/providers/odbc/hooks/odbc/index.html#module-airflow.providers.odbc.hooks.odbc

There's no mention of the OdbcHook needing an argument, odbc_conn_id="my_odbc_connection" otherwise it looks for a default of 'odbc_default'.

Even looking at the source code it's not clear because of the use of kwags and attribute setting by variable name.

Many of the values on the page are missing any docstrings and the 'See ODBC Connection for full documentation' line isn't very helpful as it isn't full documentation on using ODBC Hook.

Solving the problem

Just need to document the odbc_conn_id argument. Ideally the page would have a code example of using the hook.

The missing docstrings for values such as conn_name_attr and default_conn_name should be added.

Anything else

No response

Are you willing to submit PR?

Code of Conduct

boring-cyborg[bot] commented 2 years ago

Thanks for opening your first issue here! Be sure to follow the issue template!

potiuk commented 2 years ago

Would you like to provide such an update @isaacnorman82 ? It's easy and you can become one of the > 2100 contributors. Otherwise I mark it as good first issue and maybe someone will pick it up and implement it but the fastest way for you to help others (since you already know what you think is best and solved it for yourself) is to contribute such PR.

IT's super-easy - just open the page you want to edit in our website, click "Suggest a change on this page" (bottom right) and it will open a PR which you will be able to submit straight from GitHub UI without setting up anything else.

isaacnorman82 commented 2 years ago

I'm not against contributing, but there are a few bits to resolve:

  1. Where is the page now on tip, if you click the button that takes you to report issue/fix it is no longer found. I guess it got moved since last stable release.
  2. Is there precedent in another file that uses kwargs in airflow for docstrings? The real problem is that the variable isn't there to annotate.
  3. Is someone familiar with the code able to confirm my solution is correct before I document it? Although I guess that can happen in the PR
potiuk commented 2 years ago

I'm not against contributing, but there are a few bits to resolve:

  1. Where is the page now on tip, if you click the button that takes you to report issue/fix it is no longer found. I guess it got moved since last stable release.

It's because it comes from the dosctring of the class. So it's a bit more complex as you need to find the class and edit it there.

  1. Is there precedent in another file that uses kwargs in airflow for docstrings? The real problem is that the variable isn't there to annotate.

I think it reguires looking up through the code - so feel free to do it. No matter who does it - it can be you or anyone else, but I guess no-one knows the answer by heart - this is open-source, all code is open :). There is nothing hidden there.

  1. Is someone familiar with the code able to confirm my solution is correct before I document it? Although I guess that can happen in the PR

Yes. Discussing in PR is best. In fact we don't even require or even expect issues for those kind of changes. We are PR driven, so if you make a PR, whether it's good or not can be discussed rtght there, looking at the code of the PR.

abmkas commented 2 years ago

@potiuk I would like to contribute to this, kindly let me know if I can !!

potiuk commented 2 years ago

Sure - feel free.