eric-maynard commented on code in PR #6:
URL: https://github.com/apache/polaris-tools/pull/6#discussion_r2050892073
##########
benchmarks/src/gatling/scala/org/apache/polaris/benchmarks/simulations/ReadTreeDataset.scala:
##########
@@ -61,13 +64,31 @@ class ReadTreeDataset extends Simulation {
private val verifiedViews = new AtomicInteger()
//
--------------------------------------------------------------------------------
- // Workload: Authenticate and store the access token for later use
- //
--------------------------------------------------------------------------------
- private val authenticate = scenario("Authenticate using the OAuth2 REST API
endpoint")
- .feed(authenticationActions.feeder())
- .tryMax(5) {
- exec(authenticationActions.authenticateAndSaveAccessToken)
- }
+ // Authentication related workloads:
+ // * Authenticate and store the access token for later use every minute
+ // * Wait for an OAuth token to be available
+ // * Stop the token refresh loop
+ //
--------------------------------------------------------------------------------
+ val continuouslyRefreshOauthToken: ScenarioBuilder =
+ scenario("Authenticate every minute using the Iceberg REST API")
+ .asLongAs(_ => shouldRefreshToken.get())(
+ feed(authenticationActions.feeder())
+ .exec(authenticationActions.authenticateAndSaveAccessToken)
+ .pause(1.minute)
+ )
+
+ val waitForAuthentication: ScenarioBuilder =
+ scenario("Wait for the authentication token to be available")
+ .asLongAs(_ => accessToken.get() == null)(
+ pause(1.second)
+ )
Review Comment:
nit:m I guess we don't have a concept of an established scala style, but
this formatting looks awkward to me. I'm used to seeing either:
```
.asLongAs(_ => accessToken.get() == null)(pause(1.second))
```
Or preferably:
```
.asLongAs(_ => accessToken.get() == null) {
pause(1.second)
}
```
##########
benchmarks/src/gatling/scala/org/apache/polaris/benchmarks/simulations/ReadTreeDataset.scala:
##########
@@ -61,13 +64,31 @@ class ReadTreeDataset extends Simulation {
private val verifiedViews = new AtomicInteger()
//
--------------------------------------------------------------------------------
- // Workload: Authenticate and store the access token for later use
- //
--------------------------------------------------------------------------------
- private val authenticate = scenario("Authenticate using the OAuth2 REST API
endpoint")
- .feed(authenticationActions.feeder())
- .tryMax(5) {
- exec(authenticationActions.authenticateAndSaveAccessToken)
- }
+ // Authentication related workloads:
+ // * Authenticate and store the access token for later use every minute
+ // * Wait for an OAuth token to be available
+ // * Stop the token refresh loop
+ //
--------------------------------------------------------------------------------
+ val continuouslyRefreshOauthToken: ScenarioBuilder =
+ scenario("Authenticate every minute using the Iceberg REST API")
+ .asLongAs(_ => shouldRefreshToken.get())(
+ feed(authenticationActions.feeder())
+ .exec(authenticationActions.authenticateAndSaveAccessToken)
+ .pause(1.minute)
+ )
+
+ val waitForAuthentication: ScenarioBuilder =
+ scenario("Wait for the authentication token to be available")
+ .asLongAs(_ => accessToken.get() == null)(
+ pause(1.second)
+ )
Review Comment:
nit: I guess we don't have a concept of an established scala style, but this
formatting looks awkward to me. I'm used to seeing either:
```
.asLongAs(_ => accessToken.get() == null)(pause(1.second))
```
Or preferably:
```
.asLongAs(_ => accessToken.get() == null) {
pause(1.second)
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
