-
Notifications
You must be signed in to change notification settings - Fork 4
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
Adds non-interactive shell as execution env #31
Conversation
@@ -14,6 +14,10 @@ export const executionEnvs = { | |||
name: "GH Codespaces", | |||
}, | |||
ci: { contextKey: "ci", name: "CI" }, | |||
nonInteractiveShell: { |
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.
This is all it takes for our code to extract the non_interactive_shell
events as a separate group that's not counted towards our regular usage.
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.
Ok, I'll believe you 😄
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.
Nice, I remember when we were refactoring this, it paid off now :)!
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.
@infomiho LGTM!
One thing I would like to discuss though (and you can merge in the meantime) -> is this how we want to analyze these events? How much do we undersatnd about non-interactive (NI) shell usage? Are we being overly strict here? So non-interactive usage means somebody is running wasp
programatically, more or less. This will happen if they are running it on some server possibly -> not directly, but via soem script, right? Also in CI for sure, although I expect we will categorize taht as CI first. But what else? Whta if I created a run_wasp.sh
script and I use that to run wasp
CLI locally during development? Then it will also be non-interactive usage from what I googled/GPTed, so that is not great, we are not counting true usage then. I would like us to understand here what is the possible impact of separating non-interactive-shell usage like this. For example, I noticed that for daily stats, we get some red (>30d) users removed if we extract non-interactive usage. How is that? That is quite unexpected. That means that have been active 30 days before. They are not recognized as CI. Yet, they are non-interactive. What does that mean? Is it people using their sh script to run wasp
? Is it some kind of CI that we don't pick up as CI? Are they running Wasp in some third way? Maybe in Codespaces or replit or sometihng like taht? Although we are also recognizing those, so maybe it is something else. What do ou think about all this, what are your thoughts?
My worries are that we don't understand enough what it he impact of what we did in this PR, so would like to dsicuss it a bit. But feel free to merge PR and we can continue discussin here despite it being merged.
EDIT: NI usage is ~ programmatic usage. The problem is, we don't know if that programmatic usage is part of something compeltely automated, like CI/CD/similar, or if it is part of some other tool, be it their own script or maybe some other tool that uses Wasp in the background but could still count as real usage.
Probably for now, from waht we observed, we can treat it as automatic thing (e.g. somebody trying out servers), but we should keep observing it and every so inspecting the events - we might notice at some point that there are new patterns of usage, that are valid usage but we are excluding them, and in that case we can react and adjust our analysis. We should probably add somethign like this as a comment here in the code above this line, as note.
@@ -14,6 +14,10 @@ export const executionEnvs = { | |||
name: "GH Codespaces", | |||
}, | |||
ci: { contextKey: "ci", name: "CI" }, | |||
nonInteractiveShell: { |
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.
Nice, I remember when we were refactoring this, it paid off now :)!
NOTE: We stopped with this one for a bit till we figure out if we actually want to filter on NIS (Non Interactive Shell) env var, as it is not so clear cut as we were hoping it might be. |
Too big of a hammer for this, we will instead do something more precise. |
Fixes #24