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
Initial support for SQL Populator #63
Conversation
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.
Few minor suggestions
@@ -70,7 +80,14 @@ public RestPopulatorConfigurator withVariables(String key, String value, String. | |||
@Override | |||
public void execute() { | |||
if (urlOverride) { | |||
this.populatorService.execute(this.host, this.bindPort, Collections.unmodifiableList(this.datasets), Collections.unmodifiableMap(variables)); | |||
// TODO should runners manage uri internally? Yes let's do it in next alpha since it is an internal change |
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.
Maybe convert it to the task?
import java.util.*; | ||
|
||
/** | ||
* Implementation of NoSql DSL configurator. |
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.
Not really useful comment, but also wrong :) (copy-paste?)
|
||
@Override | ||
public void execute() { | ||
// TODO Improve this so connect and disconnect only happens once. |
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.
Convert to task / issue?
import org.jboss.arquillian.populator.core.PopulatorEnricher; | ||
|
||
/** | ||
* Implementation of Populator enricher for Sql support. |
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.
Redundant.
import java.util.Map; | ||
|
||
/** | ||
* Extends the populator service for NoSql services. Each NoSql database should implement this interface, for example one for MongoDb, another one for Redis, ... |
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.
Copy paste...
@Populator | ||
@Retention(RetentionPolicy.RUNTIME) | ||
@Target({ElementType.FIELD}) | ||
public @interface Sql { |
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.
Maybe RDBMS instead? SQL is a language
try { | ||
final JdbcDatabaseTester jdbcDatabaseTester = new JdbcDatabaseTester(driver.getName(), jdbc.toString(), username, password); | ||
|
||
// TODO add custom option to get the schema to use |
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.
Task / issue?
.map(resource -> { | ||
String type = resource.substring(resource.lastIndexOf('.')); | ||
switch (type) { | ||
case "xml": { |
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.
Later on probably strategy instead of switch + reusing what we have from Core (JSON, YAML)
Upstream @33f989a |
Short description of what this resolves:
Initial support for SQL Populator
Changes proposed in this pull request: