-
Notifications
You must be signed in to change notification settings - Fork 54
perf: Use Apify-provided environment variables to obtain PPE pricing information #483
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
base: master
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -94,6 +94,24 @@ export class ChargingManager { | |
this.purgeChargingLogDataset = configuration.get('purgeOnStart'); | ||
this.useChargingLogDataset = configuration.get('useChargingLogDataset'); | ||
|
||
if ( | ||
configuration.get('actorPricingInfo') && | ||
configuration.get('chargedEventCounts') | ||
) { | ||
this.loadPricingInfo( | ||
JSON.parse( | ||
configuration.get('actorPricingInfo'), | ||
) as ActorRunPricingInfo, | ||
configuration.get('maxTotalChargeUsd'), | ||
); | ||
this.loadChargedEventCounts( | ||
JSON.parse(configuration.get('chargedEventCounts')) as Record< | ||
string, | ||
number | ||
>, | ||
); | ||
} | ||
|
||
if (this.useChargingLogDataset && this.isAtHome) { | ||
throw new Error( | ||
'Using the ACTOR_USE_CHARGING_LOG_DATASET environment variable is only supported in a local development environment', | ||
|
@@ -117,12 +135,48 @@ export class ChargingManager { | |
return this.pricingModel === 'PAY_PER_EVENT'; | ||
} | ||
|
||
private loadPricingInfo( | ||
pricingInfo: ActorRunPricingInfo | undefined, | ||
maxTotalChargeUsd: number | undefined, | ||
) { | ||
this.pricingModel = pricingInfo?.pricingModel; | ||
|
||
// Load per-event pricing information | ||
if (pricingInfo?.pricingModel === 'PAY_PER_EVENT') { | ||
for (const [eventName, eventPricing] of Object.entries( | ||
pricingInfo.pricingPerEvent.actorChargeEvents, | ||
)) { | ||
this.pricingInfo[eventName] = { | ||
price: eventPricing.eventPriceUsd, | ||
title: eventPricing.eventTitle, | ||
}; | ||
} | ||
|
||
this.maxTotalChargeUsd = | ||
maxTotalChargeUsd ?? this.maxTotalChargeUsd; | ||
} | ||
} | ||
|
||
private loadChargedEventCounts( | ||
chargedEventCounts: Record<string, number> | undefined, | ||
) { | ||
this.chargingState = {}; | ||
|
||
for (const [eventName, chargeCount] of Object.entries( | ||
chargedEventCounts ?? {}, | ||
)) { | ||
this.chargingState[eventName] = { | ||
chargeCount, | ||
totalChargedAmount: | ||
chargeCount * (this.pricingInfo[eventName]?.price ?? 0), | ||
}; | ||
} | ||
} | ||
|
||
/** | ||
* Initialize the ChargingManager by loading pricing information and charging state via Apify API. | ||
*/ | ||
async init(): Promise<void> { | ||
this.chargingState = {}; | ||
|
||
// Retrieve pricing information | ||
if (this.isAtHome) { | ||
janbuchar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (this.actorRunId === undefined) { | ||
|
@@ -131,40 +185,25 @@ export class ChargingManager { | |
); | ||
} | ||
|
||
const run = await this.apifyClient.run(this.actorRunId).get(); | ||
if (run === undefined) { | ||
throw new Error('Actor run not found'); | ||
} | ||
|
||
this.pricingModel = run.pricingInfo?.pricingModel; | ||
|
||
// Load per-event pricing information | ||
if (run.pricingInfo?.pricingModel === 'PAY_PER_EVENT') { | ||
for (const [eventName, eventPricing] of Object.entries( | ||
run.pricingInfo.pricingPerEvent.actorChargeEvents, | ||
)) { | ||
this.pricingInfo[eventName] = { | ||
price: eventPricing.eventPriceUsd, | ||
title: eventPricing.eventTitle, | ||
}; | ||
if ( | ||
this.chargingState === undefined || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how about removing the async function getPricingInfo() {
if (
configuration.get('actorPricingInfo') &&
configuration.get('chargedEventCounts')
) return { pricingInfo: JSON.parse(), ...
const run = await this.apifyClient.run(this.actorRunId).get();
...
} This would allow us to load the pricing info + events at one point in
which is IMO cleaner than the current approach, which is conditional based on the current object state |
||
this.pricingModel === undefined | ||
) { | ||
const run = await this.apifyClient.run(this.actorRunId).get(); | ||
if (run === undefined) { | ||
throw new Error('Actor run not found'); | ||
} | ||
|
||
this.maxTotalChargeUsd = | ||
run.options.maxTotalChargeUsd ?? this.maxTotalChargeUsd; | ||
} | ||
|
||
// Load charged event counts | ||
for (const [eventName, chargeCount] of Object.entries( | ||
run.chargedEventCounts ?? {}, | ||
)) { | ||
this.chargingState[eventName] = { | ||
chargeCount, | ||
totalChargedAmount: | ||
chargeCount * (this.pricingInfo[eventName]?.price ?? 0), | ||
}; | ||
this.loadPricingInfo( | ||
run.pricingInfo, | ||
run.options.maxTotalChargeUsd, | ||
); | ||
this.loadChargedEventCounts(run.chargedEventCounts); | ||
} | ||
} | ||
|
||
this.chargingState ??= {}; | ||
|
||
if (!this.isPayPerEvent || !this.useChargingLogDataset) { | ||
return; | ||
} | ||
|
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.
nit: you have to call the new
load*
methods in this specific order, asloadChargedEventCounts
reads variables initialized byloadPricingInfo
(temporal coupling)