Skip to content

Conversation

B4nan
Copy link
Member

@B4nan B4nan commented Feb 20, 2025

The ACTOR_MAX_TOTAL_CHARGE_USD env var wasnt properly marked as integer, and the fallback to infinity wasn't working in case the value was an empty string, since ?? fallbacks only for null/undefined values.

@B4nan B4nan added the adhoc Ad-hoc unplanned task added during the sprint. label Feb 20, 2025
@github-actions github-actions bot added this to the 108th sprint - Tooling team milestone Feb 20, 2025
@github-actions github-actions bot added t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics. labels Feb 20, 2025
@B4nan B4nan requested a review from janbuchar February 20, 2025 11:12
describe('Actor.config and PPE', () => {
test('should work', async () => {
await Actor.init();
process.env.ACTOR_MAX_TOTAL_CHARGE_USD = '';
Copy link
Contributor

Choose a reason for hiding this comment

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

What about this case: process.env.ACTOR_MAX_TOTAL_CHARGE_USD = '0';? In that case we probably don't want to coerce to Infinity...

Copy link
Member Author

Choose a reason for hiding this comment

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

@mhamas thoughts? it sounds fine to me, 0 is often considered to be infinity.

if we want to distinguish empty string and zero, we'd have to adjust the logic in the config class a bit (and that would first need to happen in crawlee, but we can hotfix it from the SDK too now so we are not blocked by the crawlee release). i guess it would be fine if we just check for the empty string and map the value to undefined.

Copy link

Choose a reason for hiding this comment

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

0 is not a valid value for the total max. I think coercing to infinity is fine in that case.

describe('Actor.config and PPE', () => {
test('should work', async () => {
await Actor.init();
process.env.ACTOR_MAX_TOTAL_CHARGE_USD = '';
Copy link

Choose a reason for hiding this comment

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

0 is not a valid value for the total max. I think coercing to infinity is fine in that case.

@B4nan B4nan merged commit bb65f70 into master Feb 20, 2025
8 checks passed
@B4nan B4nan deleted the fix-charging-env-var-support branch February 20, 2025 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants