Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
adds OrientDB 3.0.x module #1414
adds OrientDB 3.0.x module #1414
Changes from 21 commits
e3798c4
1aa3606
806de50
fab67ec
01208f7
5194b0d
8dcb195
cceabb8
6d061ca
fbdf916
394eb26
7ab3988
667b666
4c4fc2a
6496d09
2d68734
321804e
4aacece
82a3bd3
7c8bcc3
6460453
e68cd4e
47d911a
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a tiny style point, but I'd be inclined to reduce the spacing and reorder some of these fields (to group similar fields together, and to reduce visible space taken).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cleaned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This username and password don't seem to be used to configure the container. In the JDBC database classes, username/password are passed through to the container via environment variables, to tell the container's bootstrap script to create that account.
It doesn't look like OrientDB supports this natively. Line 147 might be an opportunity to create the database and give access to the provided
username
, but as far as I can tell it's just expecting those credentials to work on line 150.Is that accurate?
If these methods don't ensure that the account exists, I'd prefer to remove them to avoid confusion, TBH.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I misunderstood the purpose of the member variables of the Container class, mixing server and client variables. I moved to the getSession method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this test is unnecessary, since it's a feature of Testcontainers core. Can we remove?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed