ActianCorp / actian_tableau_connector

Tableau connector (aka taco) for Actian Avalanche, Vector, and Ingres
https://extensiongallery.tableau.com/products/936
Apache License 2.0
1 stars 4 forks source link

Update to 1.0.3 with connection properties changes #21

Closed hab6 closed 1 year ago

hab6 commented 1 year ago

Updated JDBC connector code based on comments and documentation referred to in issue https://github.com/ActianCorp/actian_tableau_connector/issues/6 to better protect sensitive values (e.g. password).

Updated JDBC and ODBC connector version to 1.0.3.

The code for both tacos has been packaged, signed, and undergone limited testing, including a TDVT test suite runs.

clach04 commented 1 year ago

Quick recap of where I think we are:

hab6 commented 1 year ago

@clach04 I like your idea of abandoning this PR as there was a 7 month gap with no work due to other projects. I'll put the changes in a new PR plus clean things up based on comments in this PR.

hab6 commented 1 year ago

(For now still posting comments to this PR)

In comment, @clach04 suggested checking if we can add XML comments to files.

I tested adding comments to all of the ODBC and JDBC taco source files.

The Javascript (.js) files allow both // comment and `/ comment */` style comments.

The other (.tcd, .tdr, .tdd, .xml) files all allow XML style <!-- comment --> comments.

 connectionBuilder.js
 connectionProperties.js
 connection-dialog.tcd
 connectionResolver.tdr
 dialect.tdd
 connectionFields.xml
 connectionMetadata.xml
 manifest.xml

After adding comments to each of these files, I was able to package, sign, and use both the ODBC and JDBC connectors with Tableau Desktop.

clach04 commented 1 year ago

That's awesome that XML comments work, I was optimistic so thanks for doing the actual hard work of testing it out.

We're already using code comments for js, let's make sure we do the same for XML.

clach04 commented 1 year ago

@hab6 confirming we can close/reject this with https://github.com/ActianCorp/actian_tableau_connector/pull/26 as the accepted/submitted replacement?

hab6 commented 1 year ago

@clach04 Yes, closing this PR since #26 replaces it. Thanks.