[#226] Fix status code in GET/HEAD delete marker #242

dkirillov · 2023-10-17T11:24:18Z

dkirillov commented

2023-10-17 11:24:18 +00:00

close #226

dkirillov self-assigned this 2023-10-17 11:24:18 +00:00

dkirillov requested review from storage-services-committers 2023-10-17 11:25:07 +00:00

dkirillov requested review from storage-services-developers 2023-10-17 11:25:08 +00:00

alexvanin approved these changes 2023-10-17 14:48:56 +00:00

api/layer/object.go Outdated

					
				@ -80,2 +84,4 @@

				)

				func (e DeleteMarkerError) Error() string {

					return "object is delete marker"

alexvanin commented

2023-10-17 14:48:37 +00:00

What you think about having %v: object is delete marker formatting with inclusion of api error code. Does it make sense here or not?

What you think about having `%v: object is delete marker` formatting with inclusion of api error code. Does it make sense here or not?

dkirillov commented

2023-10-18 06:20:03 +00:00

I'm not sure that it makes a big sense. I don't know how having api error code can help us. In addition currently we log this error along with full s3 api error description (in logAndSendError function).

I'm not sure that it makes a big sense. I don't know how having api error code can help us. In addition currently we log this error along with full s3 api error description (in `logAndSendError` function).

alexvanin marked this conversation as resolved

dkirillov force-pushed bugfix/226-get_head_delete_marker from 632aa3f979 to 0dd50e4c89

2023-10-17 14:51:57 +00:00

Compare

dkirillov force-pushed bugfix/226-get_head_delete_marker from 0dd50e4c89 to fad3f13a2b

2023-10-19 14:39:57 +00:00

Compare

mbiryukova reviewed 2023-10-23 11:17:15 +00:00

api/handler/util.go Outdated

					
				@ -47,0 +52,4 @@

						return err

					}

					w.Header().Set(api.AmzDeleteMarker, "true")

mbiryukova commented

2023-10-23 11:17:15 +00:00

Is it ok that AmzDeleteMarker header may appear in other responses (e.g. copy)?

Is it ok that `AmzDeleteMarker` header may appear in other responses (e.g. copy)?

dkirillov commented

2023-10-23 13:04:12 +00:00

I think yes. I suppose it doesn't break anything because the clients probably don't handle unexpected/unknown response headers

mbiryukova marked this conversation as resolved

mbiryukova approved these changes 2023-10-23 13:28:19 +00:00

dkirillov force-pushed bugfix/226-get_head_delete_marker from fad3f13a2b to 0938d7ee82

2023-10-27 07:59:11 +00:00

Compare

alexvanin merged commit 0938d7ee82 into master

2023-10-31 13:32:26 +00:00

alexvanin deleted branch bugfix/226-get_head_delete_marker

2023-10-31 13:32:27 +00:00

Sign in to join this conversation.

No reviewers

No labels

No milestone

No project

No assignees

3 participants

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-s3-gw#242