search

pivotal-cf / azure-blobstore-resource

A concourse resource to interact with the azure blob service.
MIT License
16 stars 15 forks source link

fixed 404 bug masquerading as 400 bug. #2

Closed siennathesane closed 5 years ago

siennathesane commented 6 years ago

for reasons entirely unclear to me, when in is called, it is passed the configured payload as well as a timestamp that always ends up with "0001-01-01 00:00:00 +0000 UTC". This causes an index out of range error in Azure, which is returned up to the user as an HTTP 400 error. if the file doesn't exist, the user sees http 400 with "OutOfRangeInput".

this fix removes the snapshot functionality on Get() as there is not enough of a strong reason to snapshot a file. if needing a snapshot of a file is needed, there are much bigger problems that can't easily be addressed by concourse.

now the error message will properly return with 404 instead of 400.

hours Mike wasted on this bug: 7.

Signed-off-by: Mike Lloyd mlloyd@pivotal.io

cf-gitbot commented 6 years ago

We have created an issue in Pivotal Tracker to manage this. Unfortunately, the Pivotal Tracker project is private so you may be unable to view the contents of the story.

The labels on this github issue will be updated when the story is started.

christianang commented 6 years ago

I think the issue you ran into is that the blob you were using didn't have an initial snapshot that the resource could pick up. When you create a blob there isn't any versioning on it and you must create a snapshot explicitly to version it. I do think the UX could be a bit improved around this, in that if a snapshot isn't found it can pull the actual blob and use that or at least explicitly tell you a snapshot needs to be created, but I don't think removing the snapshot functionality is the fix.

Concourse caches older versions on the workers, but if a job tries to run with a version of the blob that no longer exists and the cache is busted or runs on a worker that never pulled the version then the job will fail. Removing snapshots in its current state will mean that you could only use the latest version of the blob, which I think would be unexpected.

christianang commented 5 years ago

I've fixed the initial snapshot issue described above in ca1ca25cd1ba671d45cb78298571d7e2a1131a03.

Closing this.