First attempt at adding a fallback option which queries graphQL should the http request fail with status 404 #915
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JulesFaucherre
approved these changes
Apr 20, 2023
elliotforbes
commented
Apr 20, 2023
| @@ -98,7 +98,6 @@ func (c *Client) DoRequest(req *http.Request, resp interface{}) (statusCode int, | |||
| }{} | |||
| err = json.NewDecoder(httpResp.Body).Decode(&httpError) | |||
| if err != nil { | |||
| fmt.Printf("failed to decode body: %s", err.Error()) | |||
There was a problem hiding this comment.
This has been deliberately removed to reduce noise when the fallback legacy path is used for config compilation and validation.
elliotforbes
commented
Apr 20, 2023
| assert.Error(t, err) | ||
| assert.Contains(t, err.Error(), "this version of the CLI does not support your instance of server") | ||
| }) | ||
| // commenting this out - we have a legacy_test.go unit test that covers this behaviour |
There was a problem hiding this comment.
deliberately commented out for now until we opt for the pinning model.
elliotforbes
commented
Apr 20, 2023
| if statusCode == 404 { | ||
| return nil, errors.New("this version of the CLI does not support your instance of server, please refer to https://github.com/CircleCI-Public/circleci-cli for version compatibility") | ||
| statusCode, originalErr := c.compileRestClient.DoRequest(req, configCompilationResp) | ||
| if statusCode == 404 { |
There was a problem hiding this comment.
If the statusCode returned from the request is 404, we want to then attempt to call the legacy GraphQL resolver path.
gabmass
approved these changes
Apr 20, 2023
elliotforbes
force-pushed
the
adding-fallback-config-compilation-and-validation-option
branch
3 times, most recently
from
d2dc832 to
13246b2
Compare
h42zhu
approved these changes
Apr 20, 2023
…es and the CLI I've created a more in-depth version compatibility matrix
JulesFaucherre
force-pushed
the
adding-fallback-config-compilation-and-validation-option
branch
from
9bbb5f5 to
d74f84e
Compare
liamclarkedev
approved these changes
Apr 20, 2023
elliotforbes
force-pushed
the
adding-fallback-config-compilation-and-validation-option
branch
from
d74f84e to
7d09d97
Compare
elliotforbes
deleted the
adding-fallback-config-compilation-and-validation-option
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist
=========
Internal Checklist
Changes
=======
Rationale
=========
The decision to opt for pinned versions of the CLI has caused some customer impact and as such, we've opted to change tact such that the config process and validate commands run the new path first and fail back to the legacy path.
Considerations
==============
We're working through testing this now with server 3 and server 4 customers.