-
Notifications
You must be signed in to change notification settings - Fork 1
Add support for Cycloid URI schema #383
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
Conversation
interpolator/parsers/fileparser.go
Outdated
| out, err := resolver.Interpolate(uri) | ||
| if err != nil { | ||
| fmt.Println(err.Error()) | ||
| err = nil |
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.
I'm not familiar with the usecase, but is there a point to resetting the err here to nil? If we need to log potential errors here, we could perhaps collect them to an array and log them in one line before returning the result.
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.
Yeah I'm still thinking about how to handle errors in this one.
I think I may need to return a list of errors to the caller indeed
fhacloid
left a comment
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.
Thanks for your review @ijozic-cycloid i'll update that this afternoon
interpolator/parsers/fileparser.go
Outdated
| out, err := resolver.Interpolate(uri) | ||
| if err != nil { | ||
| fmt.Println(err.Error()) | ||
| err = nil |
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.
Yeah I'm still thinking about how to handle errors in this one.
I think I may need to return a list of errors to the caller indeed
5abbde6 to
c8ff142
Compare
17dd220 to
26e2556
Compare
kerak19
left a comment
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.
Please fix all fmt.Errorf to provide errors as %w
|
rebase |
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.
Pull Request Overview
This PR adds support for a new Cycloid URI schema (cy://) that enables easy fetching and interpolation of resources from the Cycloid API. The implementation introduces runtime interpolation capabilities for stack files and provides a flexible way to access any API resource through URIs with shortcuts and formatting options.
- Introduces a comprehensive URI interpolation system with resolvers, formatters, and transformers
- Adds two new CLI commands:
cy uri getfor direct resource fetching andcy uri interpolatefor file processing - Implements JSON and YAML formatters with various output options and transformers for text manipulation
Reviewed Changes
Copilot reviewed 65 out of 65 changed files in this pull request and generated 8 comments.
| File | Description |
|---|---|
| interpolator/ | Core interpolation package with URI parsing, HTTP resolvers, formatters (JSON/YAML), and transformers |
| cmd/cycloid/uri/ | New CLI commands for URI operations including get and interpolate functionality |
| e2e/uri_test.go | End-to-end tests validating URI functionality and interpolation |
| Various files | Code cleanup including error message improvements, import organization, and removal of debug code |
Comments suppressed due to low confidence (1)
cmd/cycloid/uri/interpolate.go:1
- The MarkFlagsMutuallyExclusive call is incomplete - it should specify both flags that are mutually exclusive, not just one flag name.
package uri
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Btw I see many backticks weren't applied, no? |
f4afaf8 to
adb6362
Compare
kerak19
left a comment
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.
Probably last thing
This PR add support for a URI scheme able to easily fetch any resources from cycloid.
The schema is as follows:
cy://rest/path?param=valueThe path must respect the API schema, but some shortcuts has been made to make it easily writable:
There are two usecases for this feature:
cy uri interpolatecommand.cy uri getcommmand.The implementation is made as such:
The new package interpolator contains all the logic related to uri interpolation. The logic is decoupled in those components:
format=option(=value)?), available format and option parsing is implemented here. If any formatter receive a single final value (string, bool, int, etc...) it should return it as a simple string.lpad: add indentation to the output (replace helmindentfilter)nlpad: add a newline before calling lpad (replace helmnindent)base64: encode the output in base64 (useful for kubernetes secrets)