Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Jdbc containers with user, passwd, and db customizations #400

Merged
merged 1 commit into from
Sep 18, 2017
Merged

Jdbc containers with user, passwd, and db customizations #400

merged 1 commit into from
Sep 18, 2017

Conversation

nickbabcock
Copy link
Contributor

Based on our conversation in #354, all jdbc containers will allow customizations to the db, user, and password (or else throw an exception).

Since this is a different design than #354, I opened a new PR.

Closes #354

kiview
kiview requested changes Jul 18, 2017
View reviewed changes
modules/jdbc/src/main/java/org/testcontainers/containers/JdbcDatabaseContainer.java Outdated
@@ -60,6 +60,10 @@ public JdbcDatabaseContainer(String dockerImageName) {
*/
protected abstract String getTestQueryString();

public abstract SELF withUsername(String username);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are missing a default implementation here, this will break API compatibility with other JDBC modules, like https://github.com/testcontainers/testcontainers-java-module-mssqlserver. What do you think about implementing a default method throwing UnsupportedOperationException instead of abstrac method?

I see you already did this for Virtuso, just wondering how we can easily align the external modules.

@nickbabcock
Copy link
Contributor Author

Yeah have default implementations of UnsupportedOperationException will cause a lot less pain, so I have updated the PR with your suggestion 😄

@kiview
Copy link
Member

kiview commented Jul 24, 2017

Running jdbc-test seems to fail for me locally as well as on CI. Do the tests work for you?

@kiview kiview mentioned this pull request Jul 24, 2017
Closed
@nickbabcock
Copy link
Contributor Author

So I ran both the master branch and this branch a dozen or so times and every time the build failed due to a test unrelated to this PR. To me it kinda sounds like flaky tests, so I wasn't too concerned when Travis failed.

These are some of the behaviors I see:

  • Postgres connection refused
  • SimpleNginxTest fails
  • Infinite loop of "No such container: 01a881ed652b015923f2609ea1d7cc0d9542383f4f279f6d8c68031ad8e55c6b"

@nickbabcock nickbabcock force-pushed the jdbc-with branch 3 times, most recently from 4b5133b to 740b760 Compare August 30, 2017 18:50
@manikmagar
Copy link
Contributor

Any planned release date for this functionality?

@nickbabcock
Copy link
Contributor Author

Sorry, soon I hope. I've been working on it lately, but I'm still not sure what is causing the test failure. I do think it is something related to my changes, but longong and flaky tests don't make debugging any easier 😆

@nickbabcock
Copy link
Contributor Author

Tests now passing (it was a silly omission on my end 😊)

Not sure why codacy is complaining about the result set.

This PR should be good to merge.

@manikmagar
Copy link
Contributor

@nickbabcock Great! Thanks. Really, waiting for this one to be released!

I think codacy wants you to change resultSet.next() into if .. else check for "properly handling" navigation method :)

@rnorth
Copy link
Member

rnorth commented Sep 18, 2017

Argh, the codacy warning is overzealous. In a test method where we control the data, we don't need to be as fussy about this use of the JDBC API. I'll go ahead and merge. Sorry for taking so long to action this.

@rnorth rnorth merged commit 56513a4 into testcontainers:master Sep 18, 2017
rnorth added a commit that referenced this pull request Sep 18, 2017
@rnorth
Update changelog to reference #400
135da1b 
Misc wording/style changes to latest changelog entries
@manikmagar
Copy link
Contributor

Thanks @rnorth . Do you have any release date in mind for this one?

@dependabot-preview dependabot-preview bot mentioned this pull request Dec 23, 2018
Merged
rnorth pushed a commit that referenced this pull request Dec 24, 2018
@dependabot @rnorth
Bump influxdb-java from 2.10 to 2.14 in /modules/influxdb (#1070)
f986243 
Bumps [influxdb-java](https://github.com/influxdata/influxdb-java) from 2.10 to 2.14.
<details>
<summary>Changelog</summary>

*Sourced from [influxdb-java's changelog](https://github.com/influxdata/influxdb-java/blob/master/CHANGELOG.md).*

> ## 2.14 [2018-10-12]
> 
> ### Fixes
> 
> - Fixed chunked query exception handling [Issue #523](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/523)
> - Memory leak in StringBuilder cache for Point.lineprotocol() [Issue #526](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/521)
> 
> ## 2.13 [2018-09-12]
> 
> ### Fixes
> - MessagePack queries: Exception during parsing InfluxDB version [macOS] [PR #487](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/487)
> - The InfluxDBResultMapper is able to handle results with a different time precision [PR #501](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/501)
> - UDP target host address is cached [PR #502](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/502)
> - Error messages from server not parsed correctly when using msgpack [PR #506](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/506)
> - Response body must be closed properly in case of JSON response [PR #514](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/514)
> - Time is serialized not consistently in MsgPack and Json, missing millis and nanos in MsgPack[PR #517](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/517)
> 
> ### Features
> 
> - Support for Basic Authentication [PR #492](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/492)
> - Added possibility to reuse client as a core part of [influxdb-java-reactive](https://github.com/bonitoo-io/influxdb-java-reactive) client [PR #493](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/493)
> - Retry capability for writing of BatchPoints [PR #503](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/503)
> - Added `BiConsumer` with capability to discontinue a streaming query [Issue #515](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/515)
> - Added `onComplete` action that is invoked after successfully end of streaming query [Issue #515](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/515)
> 
> ## 2.12 [2018-07-31]
> 
> ### Fixes
> 
> - Remove code which checks for unsupported influxdb versions [PR #474](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/474)
> - Unpredictable errors when OkHttpClient.Builder instance is reused [PR #478](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/478)
> 
> ### Features
> 
> - Support for MessagePack [PR #471](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/471)
> - Cache version per influxdb instance and reduce ping() calls for every query call [PR #472](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/472)
> - FAQ list for influxdb-java [PR #475](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/475)
> 
> ### Improvements
> 
> - Test: Unit test to ensure tags should be sorted by key in line protocol (to reduce db server overheads) [PR #476](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/476)
> 
> ## 2.11 [2018-07-02]
> 
> ### Features
> 
> - Allow write precision of TimeUnit other than Nanoseconds [PR #321](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/321)
> - Support dynamic measurement name in InfluxDBResultMapper [PR #423](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/423)
> - Debug mode which allows HTTP requests being sent to the database to be logged [PR #450](https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/450)
> - Fix problem of connecting to the influx api with URL which does not points to the url root (e.g. localhots:80/influx-api/) [PR #400] (https://github-redirect.dependabot.com/influxdata/influxdb-java/pull/400)
></table> ... (truncated)
</details>
<details>
<summary>Commits</summary>

- [`91d0f09`](influxdata/influxdb-java@91d0f09) [maven-release-plugin] prepare release influxdb-java-2.14
- [`8ffaeb9`](influxdata/influxdb-java@8ffaeb9) Revert "[maven-release-plugin] prepare release influxdb-java-2.14"
- [`2781da2`](influxdata/influxdb-java@2781da2) [maven-release-plugin] prepare release influxdb-java-2.14
- [`19c69ed`](influxdata/influxdb-java@19c69ed) [maven-release-plugin] prepare for next development iteration
- [`c6d7f25`](influxdata/influxdb-java@c6d7f25) [maven-release-plugin] prepare release influxdb-java-2.14
- [`2f4c594`](influxdata/influxdb-java@2f4c594) Merge pull request [#531](https://github-redirect.dependabot.com/influxdata/influxdb-java/issues/531) from heshengbang/master
- [`f653e62`](influxdata/influxdb-java@f653e62) Easy to use try-with-resources, add README.md
- [`c7be9b0`](influxdata/influxdb-java@c7be9b0) Easy to use try-with-resources
- [`4590d18`](influxdata/influxdb-java@4590d18) - added automated SNAPSHOT publishing to Maven Central repository
- [`ce65a41`](influxdata/influxdb-java@ce65a41) - added automated SNAPSHOT publishing to Maven Central repository
- Additional commits viewable in [compare view](influxdata/influxdb-java@influxdb-java-2.10...influxdb-java-2.14)
</details>
<br />

[![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=org.influxdb:influxdb-java&package-manager=gradle&previous-version=2.10&new-version=2.14)](https://dependabot.com/compatibility-score.html?dependency-name=org.influxdb:influxdb-java&package-manager=gradle&previous-version=2.10&new-version=2.14)

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

**Note:** This repo was added to Dependabot recently, so you'll receive a maximum of 5 PRs for your first few update runs. Once an update run creates fewer than 5 PRs we'll remove that limit.

You can always request more updates by clicking `Bump now` in your [Dependabot dashboard](https://app.dependabot.com).

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in the `.dependabot/config.yml` file in this repo:
- Update frequency (including time of day and day of week)
- Automerge options (never/patch/minor, and dev/runtime dependencies)
- Pull request limits (per update run and/or open at any time)
- Out-of-range updates (receive only lockfile updates, if desired)
- Security updates (receive only security updates, if desired)

Finally, you can contact us by mentioning @dependabot.

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kiview kiview kiview approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@nickbabcock @kiview @manikmagar @rnorth