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.

Sign up for GitHub

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

Jump to bottom

Ignore 404 in fetch_active_workflows [semver:minor] #80

Closed
wants to merge 1 commit into from
Closed

Ignore 404 in fetch_active_workflows [semver:minor] #80

wants to merge 1 commit into from

Conversation

davet1985
Copy link

@davet1985 davet1985 commented Apr 27, 2022 •
edited
Loading

Checklist

  • All new jobs, commands, executors, parameters have descriptions
  • Examples have been added for any significant new features
  • README has been updated, if necessary

Motivation, issues

Attempts to fix #79 by better handling a 404 from the CircleCI API when getting workflows.

Description

@davet1985 davet1985 marked this pull request as ready for review April 27, 2022 13:18
@davet1985
Handle 404 in fetch_active_workflows
b550694 
- removing -f from curl command in fetch
- this was causing the command to exit with code 22 upon getting a 404
- check the file containing the response for `created_at` before continuing
@davet1985 davet1985 mentioned this pull request Apr 27, 2022
Closed
@davet1985 davet1985 closed this Apr 27, 2022
@davet1985 davet1985 reopened this Apr 27, 2022
@asselinpaul
Copy link

@eddiewebb would be great to have this merged

@eddiewebb
Copy link
Owner

To move forward with this we need to make it optional, as a parameter to ignore missing workflow data, or figure out why IDs declared by active jobs don't exist.

ignore-missing-worflows or similar with a default behavior to expect all data, and an override to allow missing data to just be an notice and not a failure.

@eddiewebb eddiewebb changed the title Handle 404 in fetch_active_workflows Ignore 404 in fetch_active_workflows [semver:minor] Apr 28, 2022
@asfaltboy
Copy link

@eddiewebb given your recent findings, do we still want this to be merged, maybe as a workaround at least? Does it depend on adding a parameter to toggle this feature?

@eddiewebb
Copy link
Owner

eddiewebb commented May 4, 2022 •
edited
Loading

@asfaltboy, @davet1985, @asselinpaul - I closed #79 based on a fix and recent API changes.

If we want to push for this PR anyway I would want to gate it behind a parameter like ignore-404 with a default to NOT ignore them as they could actually be 403/bad token errors.

@eddiewebb
Copy link
Owner

I am closing for now. If there is interwst to re-submit feature behind a flag please feel free to re-open or reference in new PR.

@eddiewebb eddiewebb closed this Oct 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[1.7.0] CircleCI API 404 response kills the job
4 participants
@davet1985 @asselinpaul @eddiewebb @asfaltboy