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

chore(consume): fix --input when running from hive #701

Merged
merged 2 commits into from
Jul 23, 2024

Conversation

spencer-tb
Copy link
Collaborator

@spencer-tb spencer-tb commented Jul 23, 2024

🗒️ Description

This PR allows us to run consume commands using the following Dockerfile entrypoint from hive:

ENTRYPOINT ["consume", "engine", "--input=https://.../execution-spec-tests/releases/.../fixtures.tar.gz", "-v"]

Problem

The condition if not sys.stdin.isatty(): checks if the standard input (stdin) is connected to a terminal. If it's not connected to a terminal, it implies that the input might be coming from a pipe or a file, which is why the command adds --input=stdin.

In a Docker container, sys.stdin.isatty() returns False because Docker doesn't provide a terminal interface by default. This causes an issue where the input is incorrectly automatically set to stdin when running in Docker.

Solution

To fix the latter we additionally check for --input within the consume command arguements. We do not supply this when piping.

🔗 Related Issues

N/A

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.
  • All: Considered updating the online docs in the ./docs/ directory.

@spencer-tb spencer-tb added type:chore Type: Chore scope:consume Scope: Consume command suite labels Jul 23, 2024
@spencer-tb spencer-tb requested a review from marioevz July 23, 2024 15:43
Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

LGTM, just the potential typo in the whitelist file.

@marioevz marioevz merged commit c332ee1 into ethereum:main Jul 23, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:consume Scope: Consume command suite type:chore Type: Chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants