• Steve Azzopardi's avatar
    Revert 9e1d0676 · 0b4538cc
    Steve Azzopardi authored
    After discussing this a bit more with fabiopitino there seems like
    there was a bit of miscommunication. We want to have the 422 error not
    retry the job but still consider it a valid response that the artifact
    was uploaded. From an API perspective, this is not the best solution
    since a 4xx status code is an error and should be treated as such.
    In light of recent events such as
    https://gitlab.com/gitlab-org/gitlab/issues/199764 we've seen spikes of
    similar behavior as https://gitlab.com/gitlab-org/gitlab/issues/36516
    (which was what this issue was trying to solve). We decided that the
    following should happen:
    1. Implement https://gitlab.com/gitlab-org/gitlab/-/merge_requests/24165
       which will handle the scenario where rails actually upload the artifact
       but fail to save the metadata or any other part. If there is a failure
       Rails would respond with a 4xx/5xx error accordingly and the Runner will
       retry to upload (this is already done) on the second try of uploading
       the artifact rails will check if artifact is already uploaded if it is,
       it will pick up from where it fails (for example storing the metadata)
    1. Revert
    because of two things
        - It does not implement the original behavior we wanted. We wanted
          to have 422 act as a valid response and not to fail the build.
        - We don't want to have any special behavior for 422, if rails
          return 422 we still want to retry the upload.
    This is not a full revert of 9e1d0676
    because it included some refactoring. This just removes the knowledge of
    the `422` error.
artifacts_uploader.go 3.53 KB