Skip to content
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

Add logging to confirm that stdout isn't empty when triggering on cell execution #333

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

jlewi
Copy link
Owner

@jlewi jlewi commented Nov 5, 2024

  • The purpose of this PR is to add logging to confirm that Trigger Foyle Ghost Cell Updates When Cell Output Changes / Cell Is Executed #309 is fixed; i.e. that when triggering on cell execution for interactive (and non interactive cells) the output is included in the request

  • This PR adds logging that checks if the selected cell has non-empty stdout if its empty we log a message. The presence of these log messages thus indicates cell execution triggered completion that included no stdout.

  • This will include false positives because in certain cases the command may not produce any output.

  • With this logging I was unable to reproduce any instances where completion was triggered by cell execution but stdout wasn't included.

  • This PR fixes a bug in the mimetype for stdout; there was an extra space. I don't think that actually impacted processing because that constant was only used in unittests before this PR.

  • fix Trigger Foyle Ghost Cell Updates When Cell Output Changes / Cell Is Executed #309

…l execution.

* The purpose of this PR is to add logging to confirm that #309 is fixed;
  i.e. that when triggering on cell execution for interactive (and non interactive cells)
  the output is included in the request

* This PR adds logging that checks if the selected cell has non-empty stdout
  if its empty we log a message. The presence of these log messages thus
  indicates cell execution triggered completion that included no stdout.

* This will include false positives because in certain cases the command may not
  produce any output.

* With this logging I was unable to reproduce any instances where completion was
  triggered by cell execution but stdout wasn't included.

* This PR fixes a bug in the mimetype for stdout; there was an extra space.
  I don't think that actually impacted processing because that constant was
  only used in unittests before this PR.

* fix #309
Copy link
Contributor

@standard-input standard-input bot left a comment

Choose a reason for hiding this comment

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

No issues flagged.
Standard Input can make mistakes. Check important info.

Copy link
Contributor

@standard-input standard-input bot left a comment

Choose a reason for hiding this comment

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

No issues flagged.
Standard Input can make mistakes. Check important info.

@jlewi jlewi changed the title Add logging to confirm that stdout isn't empty when triggering on cel… Add logging to confirm that stdout isn't empty when triggering on cell execution Nov 5, 2024
Copy link

netlify bot commented Nov 5, 2024

Deploy Preview for foyle canceled.

Name Link
🔨 Latest commit cb524e4
🔍 Latest deploy log https://app.netlify.com/sites/foyle/deploys/67297d7c5aaf60000808e0c8

@jlewi jlewi merged commit 7515a60 into main Nov 5, 2024
5 checks passed
@jlewi jlewi deleted the jlewi/interactiveexec branch November 5, 2024 02:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Trigger Foyle Ghost Cell Updates When Cell Output Changes / Cell Is Executed
1 participant