Skip to content

Conversation

@pczuj
Copy link
Contributor

@pczuj pczuj commented Feb 15, 2023

virtual-users consumers use the VirtualUserBehavior and VirtualUserOptions classes for serializing VU options. Requiring to set Scenatio as Class artificially enforces the consumer to depend on the VU JAR classes.

@pczuj pczuj requested a review from a team as a code owner February 15, 2023 12:49
@pczuj
Copy link
Contributor Author

pczuj commented Feb 15, 2023

Could do the same for

    internal val userGenerator: Class<out UserGenerator>,
    internal val logging: Class<out AbstractConfiguration>

but nto sure if it's worth.

@pczuj pczuj force-pushed the issue/JPERF-996-allow-not-visible-scenario branch from c333b48 to b1dd750 Compare February 15, 2023 12:55
…path

virtual-users consumers use the VirtualUserBehavior and VirtualUserOptions classes for serializing VU options.
Requiring to set Scenatio as Class artificially enforces the consumer to depend on the VU JAR classes.
@pczuj pczuj force-pushed the issue/JPERF-996-allow-not-visible-scenario branch from b1dd750 to c1bcc6e Compare February 15, 2023 13:05
@pczuj
Copy link
Contributor Author

pczuj commented Feb 15, 2023

Simplified it a bit more. Turned out that after my changes we didn't need getScenario private method of VirtualUserOptions

private val scenario = behavior.scenario.getConstructor().newInstance() as Scenario
private val scenario = Class.forName(behavior.scenarioClass).getConstructor().newInstance() as Scenario
private val browser = behavior.browser.getConstructor().newInstance() as Browser
private val userGenerator = options.behavior.userGenerator.getConstructor().newInstance() as UserGenerator
Copy link
Contributor

Choose a reason for hiding this comment

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

Wanna do the same for UserGenerator for consistency?

@dagguh
Copy link
Contributor

dagguh commented Feb 15, 2023

WRT CI:

java.lang.Exception: Error while executing lftp -c 'set net:timeout 15; set net:max-retries 50; pget -n 32 -c "https://s3-eu-central-1.amazonaws.com/jpt-custom-datasets-storage-a008820-datasetbucket-dah44h6l1l8p/jsw-7.13.0-100k-users-sync/database.tar.bz2" -o database.tar.bz2'. Exit status code SshResult(exitStatus=1, output=, errorOutput=pget: Access failed: 404 Not Found (/jpt-custom-datasets-storage-a008820-datasetbucket-dah44h6l1l8p/jsw-7.13.0-100k-users-sync/database.tar.bz2)

This was fixed in infrastructure:4.22.4.

@dagguh
Copy link
Contributor

dagguh commented Feb 15, 2023

Could do the same for

internal val userGenerator: Class<out UserGenerator>,
internal val logging: Class<out AbstractConfiguration>

but nto sure if it's worth.

Yeah, it's gonna be a WTF factor.

@pczuj
Copy link
Contributor Author

pczuj commented Feb 21, 2023

Do you think this is the right direction of the code? In fact it's not currently necessary, however I thought that it will be technically better and would simplify some dependency sets. It could also in some cases allow for 2 conflicting classpaths between VU JAR and test code to coexist (you don't need to depend on the VU JAR).

@pczuj pczuj marked this pull request as draft February 28, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants