Skip to content

Conversation

@tw4l
Copy link
Member

@tw4l tw4l commented Oct 23, 2023

Fixes #1306

  • Include full resources with expireAt (as string) in crawlFinished and uploadFinished webhook notifications rather than using the downloadUrls field (this is retained for collections).
  • Set default presigned duration to just short of 1 week and enforce maximum supported by S3
  • Update tests

Tested locally and on dev

- Include full resources with expireAt (as string) in crawlFinished
and uploadFinished webhook notifications rather than using the
downloadUrls field (this is retained for collections).
- Set default presigned duration to just short of 1 week and
enforce maximum supported by S3
- Update tests
self.presign_duration_seconds = (
int(os.environ.get("PRESIGN_DURATION_MINUTES", 60)) * 60
presign_duration_seconds = (
int(os.environ.get("PRESIGN_DURATION_MINUTES", PRESIGN_MINUTES_DEFAULT))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We also have:

  PRESIGN_DURATION_MINUTES: "{{ .Values.storage_presign_duration_minutes | default 60 }}"

in configmap.yaml so that will take precedence. probably should keep this default in one place only to avoid confusion, can make the env var blank by default

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, I see its been updated to latest value, but should it be blank there perhaps?

- set default minutes in code, if omitted in chart
- add commented out value to chart
- use minutes as maximum
@ikreymer ikreymer merged commit d58747d into main Oct 24, 2023
@ikreymer ikreymer deleted the issue-1306-webhook-download-url branch October 24, 2023 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Download URL in Webhook payload incorrectly prepends

3 participants