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

Print full path rather than RelativePath only. #903

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

ernstvonoelsen
Copy link
Contributor

@ernstvonoelsen ernstvonoelsen commented Apr 8, 2024

targets resolution of /issues/895

This PR just switches to print each file's Description() rather than RelativePath(). In order to keep quoting consistent, some single quotes were removed.

Signed-off-by: Ernst von Oelsen <ernst.vonoelsen@tngtech.com>
@ernstvonoelsen
Copy link
Contributor Author

This is my first contribution to this project, I learned about it on last month's CloudNativeCon & KubeCon Europe in Paris.

I'm happy to receive any feedback on how to improve it.

Signed-off-by: Ernst von Oelsen <ernst.vonoelsen@tngtech.com>
@renuy
Copy link

renuy commented Apr 19, 2024

Hey @ernstvonoelsen , thanks for this.
@prembhaskal will help with review of this PR.

@prembhaskal
Copy link
Member

@ernstvonoelsen Could you please post some testing output if you have done? Does the description prints the absolute path now?
I thought the main issue in #895 was that relative path calculation was incorrect when individual files with different path, but same name are input to ytt.

@ernstvonoelsen
Copy link
Contributor Author

@prembhaskal

console output from develop branch:

$ tree
.
├── directory_1
│   └── values.yaml
└── directory_2
    └── values.yaml

2 directories, 2 files
$ ytt -f directory_1/values.yaml -f directory_2/values.yaml 
ytt: Error: Unmarshaling YAML template 'values.yaml': yaml: line 3: could not find expected ':'

same test directory, but from issue-895 branch:

$ ytt -f directory_1/values.yaml -f directory_2/values.yaml 
ytt: Error: Unmarshaling YAML template 'file directory_1/values.yaml': yaml: line 3: could not find expected ':'

when called from one of the sub directories:

$ pwd
directory_1

$ ytt -f values.yaml -f ../directory_2/values.yaml 
ytt: Error: Unmarshaling YAML template 'file values.yaml': yaml: line 3: could not find expected ':'
$ pwd
directory_2

$ ytt -f values.yaml -f ../directory_1/values.yaml 
ytt: Error: Unmarshaling YAML template 'file ../directory_1/values.yaml': yaml: line 3: could not find expected ':'

From what I conclude from the existing code, each func Description() implementation in sources.go returns either

  • s.path for BytesSource
  • "stdin.yml" for StdinSource
  • fmt.Sprintf("file %s", s.path) for LocalSource
  • fmt.Sprint("HTTP URL %s, s.url) for HTTPSource

So Description() does not print the absolute path of each input file, but at least it prints a unique path to the invalid YAML file..

@renuy renuy requested a review from cppforlife May 17, 2024 06:24
Copy link
Member

@joaopapereira joaopapereira left a comment

Choose a reason for hiding this comment

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

LGTM

@joaopapereira joaopapereira merged commit 91690b6 into carvel-dev:develop Jun 7, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants