Allow hostname to be provided with or without a trailing slash

[tim-steinkuhler]

resolves #302

Description

Allow hostname to be provided with or without trailing slash

When you copy in the hostname, it may or may not include trailing /
We could avoid confusion by allowing users to include or exclude these elements.

Before this change, dbt debug would get you something like this:

dbt was unable to connect to the specified database.
The database returned the following error:

  >Runtime Error
  Database Error
    failed to connect

Output while using a trailing slash after change (given credentials are correct):

Connection test: [OK connection ok]

Checklist

• I have read the contributing guide and understand what's expected of me
• I have signed the CLA
• I have run this code in development and it appears to resolve the stated issue
• This PR includes tests, or tests are not required/relevant for this PR
• I have opened an issue to add/update docs, or docs changes are not required/relevant for this PR
• I have run changie new to create a changelog entry

[cla-bot]

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: Tim Steinkühler.
This is most likely caused by a git client misconfiguration; please make sure to:

1. check if your git client is configured with an email to sign commits git config --list | grep email
2. If not, set it up using git config --global user.email [email protected]
3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

[tim-steinkuhler]

Hi @colin-rogers-dbt (I see you are assigned as reviewer), I'm finding it a bit difficult to relate the CI error message "at least one column must be specified for the table" to the changes I made. If I did mess this up, I would expect a lot more tests to fai, not just that one, and I would expect a different error message. Could you help me get a better understanding?

[tim-steinkuhler]

@Fokko, are you able to have a look at this? After a rebase on the latest version of main, the tests have been passed, although there are still 3 "expected" checks, how does this work? :-D

[Fokko]

@tim-steinkuhler A maintainer (someone from dbt-labs) has to kick off the jobs. This is to avoid someone injecting malicious code into the CI. This isn't needed anymore once one PR is in. Unfortunately, it is a bit quiet on the dbt-spark repository. I'm also trying to get a few PRs in. @VersusFacit would you have time to dig into this?

[Fokko]

Keep in mind that by changing this, we cannot use http:// prefixes anymore. IDK if anyone uses this (maybe in test environments?).

[tim-steinkuhler]

Hmm, wasn't that already the case before, with this code?

                    # Prepend https:// if it is missing
                    host = creds.host
                    if not host.startswith("https://"):
                        host = "https://" + creds.host

I tried to "standardize" the host name in SparkCredentials. __post_init__(), so it wouldn't matter if you added the prefix and / or the trailing /, and then added this here, because you had the https:// prefix for the http connection method

[tim-steinkuhler]

By the way, thank you @Fokko! Your feedback is much appreciated!

[tim-steinkuhler]

I don't think my change could lead to this error message in the integration-spark-session pipeline. @colin-rogers-dbt could you have a look?

[JCZuurmond]

@tim-steinkuhler : Could you force a new commit to restart the CI? I do not understand why the integration-spark-session CI fails; I would like to rerun the CI to be sure

[JCZuurmond]

@tim-steinkuhler : Ok, that did not do the trick unfortunately. Let's discuss an approach for this in person!

[tim-steinkuhler]

After a short discussion with @JCZuurmond , we decided to not remove the https:// prefix

[JCZuurmond]

@colin-rogers-dbt : Could you help with the CI again? Given the code changes and that the CI passed before, I expect that the CI fails because it is a bit flacky.

@Fleid: This PR is ready to be merged. Could you merge it once the CI is fixed?

[colin-rogers-dbt]

Thanks for the contribution!