Update db2_data_source.py

[4rahulae]

This is needed in order to fix the connectivity issues for db2 from soda-core.

Please add this change as part of the soda-core release

[CLAassistant]

All committers have signed the CLA.

[4rahulae]

once the approvals are done and the changes are approved, how soon can this be added to the new release?

[4rahulae]

changes look good to me, adding this self.verify_ssl = data_source_properties.get("security") in line 68 and updating conn_str in line 72 or 73 as below
conn_str = f"DATABASE={self.database};HOSTNAME={self.host};PORT={self.port};UID={self.username};PWD={self.password};SECURITY={self.verify_ssl}"

[4rahulae]

@m1n0 @vijaykiran Hi Milan as per our discussions earlier can you please review this PR and approve it and merge it for release?

[dirkgroenen]

Hi @4rahulae, thanks for your contribution 🙌 @m1n0 is currently traveling back from our company offsite, so he'll likely not reach out today.

I'm myself not able to test this at the moment, but could you assist by verifying that your changes do not impact configurations where security is not set? According to the documentation I can see that it only accepts SSL as a valid value, so I would like to make sure this does not introduce backwards compatability issues for anyone who does not define security in their configuration.yml.

(For example a possible scenario could be that Python translates it into an empty string or None, triggering DB2 to fail)

[4rahulae]

Hi @dirkgroenen i did verify the backward compatibility by not defining security in the configuration.yml and the scan doesn't fail except that it returns with a message "[IBM][CLI Driver] SQL30081N A communication error has been detected" which only applies to my use case and after adding security: SSL it works. I believe in other scenarios when security is not defined it will default to None or empty string and if defining security is not needed it should still work

[m1n0]

thanks for the contrib @4rahulae ! I would suggest a bit more conservative way to do this though - pythons get without a second param returns NULL, so the VERIFY_SSL= will contain nothing, I would suggest only appending it to the connection string if it does have a non-null value, would you agree?
Also, I think it would make sense to match the yaml key we read with the connection string argument, so read verify_ssl instead of security if that makes sense.

[4rahulae]

Hi @m1n0 are you suggesting doing something like below?.

self.verify_ssl = data_source_properties.get("verify_ssl","SSL")
conn_str = (
f"DATABASE={self.database};HOSTNAME={self.host};PORT={self.port};UID={self.username};PWD={self.password};SECURITY={self.verify_ssl}.

This way there is no need to define security in the config file , i have tested this and it works.
Without passing security either in the config or in db2_data_source.py as shown above I'm unable to connect to db2.

Please suggest if you have other ways to make this work as of right now i see this is the only other option that works apart from the suggested option earlier as part of pull request?

[m1n0]

hi, I don't know what the default value for SECURITY is, so maybe I would suggest a more safe/generic way to do this:

self.security = data_source_properties.get("security")

conn_str =  f"DATABASE={self.database};HOSTNAME={self.host};PORT={self.port};UID={self.username};PWD={self.password}"

if self.security is not None:
  conn_str += f";SECURITY={self.security}"

what do you think?

[4rahulae]

Hi @m1n0 added changes as per your suggestion and tested them successfully on my local, please review the changes and let me know if it looks good to be merged and added for release?

[m1n0]

Thanks for the contrib, merging!

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud