Add reflection caching for some of the methods in HANAHDBCLIDialect

[Masterchen09]

Is there any specific reason that some of the methods needed for reflection do not use caching - some of the methods already use caching (e.g., get_table_names), but some others not?

Based on the built-in dialect for PostgreSQL, the methods get_temp_table_names, get_view_names, get_view_definition, get_columns, get_foreign_keys, get_indexes, get_pk_constraint, get_unique_constraints, get_check_constraints, get_table_oidand get_table_commentcould also make use of the reflection caching - this PR adds the corresponding decorators for it. :-)

[kasium]

The methods on which the caching is used contains then one of https://docs.sqlalchemy.org/en/20/changelog/whatsnew_20.html#behavioral-changes-for-inspector and the ones covered by SQLAlchemy tests but I guess it won't hurt to have it on more methods

[kasium]

Seems like there is a CI issue. Let me check this later

[kasium]

@Masterchen09 I checked the CI and it seems to be caused by the secret/fork combination. In order to connect to our SAP HANA instance used for testing a secret containing the connection details is passed to the test script. However, github runners prevent this mechanism for security reasons when a fork is used.

Could you please try to do this w/o a fork, so that you create a branch in this repo directly? I would do this but I must know if any protection rule prevents this 😄 . If it works, I'll update the README. I'm very sorry for the inconvenience

[Masterchen09]

It seems I don't have the permission to create a new branch directly in the repository - I guess you have to do it. 😊

[kasium]

@Masterchen09 done: https://github.com/SAP/sqlalchemy-hana/tree/feat-enable-cache

[Masterchen09]

I can also not push to the newly created branch...😅

I mean if anyone would be able to push to a branch in the repository, it would be possible to bypass the security mechanism, when the pipeline is automatically running then...somehow it should also work with forks, because I guess that's how you work on GitHub? As long as a maintainer/you have to approve pipelines before they are running it should also be safe regarding the credentials (i.e. before running the pipeline you have to make sure that the credentials are not printed somewhere for example...or is in GitHub also a variable masking mechanism available like it is in GitLab? Then it should not be an security issue at all, when the credentials are always masked in the log?).

[kasium]

You are right, but for reasons only GitHub knows secrets are not accessible when the PR comes from a fork. However, there is maybe a way. Let me check this 😄

[kasium]

@Masterchen09 solved it. We are now using pull_request_target and some action security features. As far as I understood the procedure you need to do the following:

• fetch the latest upstream main into your fork
• create a new PR based on the latest main
• wait for my approval to run the CI

As said, I'm very sorry for the complicated setup. Your contribution is highly appreciated!

[Masterchen09]

It seems that a new PR was not necessary: Rebasing was sufficient and the CI pipelines are now working. I guess you didn't had to approve it this time, or you were so quick with the approval that I didn't noticed it - maybe this is something you want to check again in case it is security relevant (or it was intentionally).

P.S.: I am happy to play the guinea pig ("Versuchskaninchen") and have absolutely no problems with it. 😁