Skip to content
This repository has been archived by the owner on Jul 12, 2022. It is now read-only.

Add stdin validation for docker execution #493

Merged
merged 8 commits into from
Sep 3, 2020
Merged

Add stdin validation for docker execution #493

merged 8 commits into from
Sep 3, 2020

Conversation

GuillaumeFalourd
Copy link
Contributor

Signed-off-by: GuillaumeFalourd guillaume.falourd@zup.com.br

- What I did

  • Validate if the stdin flag has been used when running a command with Docker.

- How to verify it

  • Building locally and executing the following command :
echo '{"input_text":"Dennis", "input_bool":"false", "input_list":"everything", "input_password":"Ritchie"}' | ./rit demo hello-world --stdin --docker

- Description for the changelog

  • Validate if the stdin flag has been used when running a command with Docker.

Signed-off-by: GuillaumeFalourd <guillaume.falourd@zup.com.br>
@GuillaumeFalourd GuillaumeFalourd linked an issue Sep 2, 2020 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Sep 2, 2020

Codecov Report

Merging #493 into master will not change coverage.
The diff coverage is 0.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #493   +/-   ##
=======================================
  Coverage   74.74%   74.74%           
=======================================
  Files          95       95           
  Lines        3184     3184           
=======================================
  Hits         2380     2380           
  Misses        640      640           
  Partials      164      164           
Impacted Files Coverage Δ
pkg/formula/runner/docker/runner.go 90.56% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7bf3935...933f8f8. Read the comment docs.

@GuillaumeFalourd GuillaumeFalourd added 🪲 bug Report a bug encountered while operating Ritchie ✔️ ready-for-review ready for review labels Sep 2, 2020
@GuillaumeFalourd GuillaumeFalourd changed the title [BUG] Add stdin validation for docker execution Add stdin validation for docker execution Sep 2, 2020
@GuillaumeFalourd GuillaumeFalourd self-assigned this Sep 2, 2020
Copy link
Contributor

@brunasilvazup brunasilvazup left a comment

Choose a reason for hiding this comment

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

We have a pr in qa that will change a lot about the runner > #480
I recommend that you wait and assess whether the error will persist.

@brunasilvazup brunasilvazup added 🕒 Wait and removed ✔️ ready-for-review ready for review labels Sep 2, 2020
@GuillaumeFalourd
Copy link
Contributor Author

GuillaumeFalourd commented Sep 2, 2020

We have a pr in qa that will change a lot about the runner > #480
I recommend that you wait and assess whether the error will persist.

I've checked the PR @brunasilvazup and this problem hasn't been resolved there.
It's related to the args defined when running docker, and the feature implemented by @kaduartur didn't update that section.

I can eventually update Kadu's branch with that correction if necessary.

Copy link
Contributor

@kaduartur kaduartur left a comment

Choose a reason for hiding this comment

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

LGTM!

@brunasilvazup brunasilvazup added 🚧 WIP Work in Progress and removed 🕒 Wait labels Sep 2, 2020
@kaduartur
Copy link
Contributor

/merge qa

@ritchie-bot
Copy link
Contributor

ritchie-bot bot commented Sep 2, 2020

🔥 Merge Conflict

@kaduartur
Copy link
Contributor

/merge qa

@ritchie-bot
Copy link
Contributor

ritchie-bot bot commented Sep 2, 2020

👌 Merged branch bug/stdin-docker into qa

@GuillaumeFalourd GuillaumeFalourd removed the 🚧 WIP Work in Progress label Sep 3, 2020
Copy link
Contributor

@brunasilvazup brunasilvazup left a comment

Choose a reason for hiding this comment

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

Tested in qa, the error has been fixed 👍

@kaduartur kaduartur merged commit c3ba037 into ZupIT:master Sep 3, 2020
@GuillaumeFalourd GuillaumeFalourd deleted the bug/stdin-docker branch May 6, 2021 17:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🪲 bug Report a bug encountered while operating Ritchie
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] STDIN & DOCKER flag don't work together
5 participants