Skip to content

Comments

Stop Using Custom Strfry Image#98

Closed
abh3po wants to merge 3 commits intomainfrom
move-to-strfry
Closed

Stop Using Custom Strfry Image#98
abh3po wants to merge 3 commits intomainfrom
move-to-strfry

Conversation

@abh3po
Copy link
Contributor

@abh3po abh3po commented Jun 24, 2025

No description provided.

@abh3po abh3po changed the title Move to strfry Stop Using Custom Strfry Image Jun 24, 2025
@abh3po abh3po requested a review from brh28 June 24, 2025 14:06
Copy link
Collaborator

@brh28 brh28 left a comment

Choose a reason for hiding this comment

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

Few comments, but nothing critical

Copy link
Collaborator

Choose a reason for hiding this comment

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

Non-blocking but I imagine it would be more convenient to have a package.json as a file next to the whitelist.js, but if it works I'm fine with it

input: { npub: npub },
};

console.error(
Copy link
Collaborator

Choose a reason for hiding this comment

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

change to info message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the strfry program looks for stdout messages, error becoems the only way to log to be able to see it while debugging, can send it to opentelemetry though

console.error("API response:", response.data); // Log response to stderr
return response.data.data.isFlashNpub.isFlashNpub;
} catch (error) {
console.error("Error fetching whitelist status:", error.message);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ideally we should add open-telemetry traces

- name: package-json
mountPath: /app/package.json
subPath: package.json
resources:
Copy link
Collaborator

Choose a reason for hiding this comment

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

this seems fairly small. Not sure the load we're expecting here, but may be useful to either increase the limit or monitor resource usage

@abh3po abh3po added the low label Sep 8, 2025
@abh3po
Copy link
Contributor Author

abh3po commented Feb 12, 2026

closing in favor of #139

@abh3po abh3po closed this Feb 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants