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.

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

ref: cleanup binary for self-hosted #284

Closed
wants to merge 3 commits into from

Conversation

aldy505
Copy link
Contributor

@aldy505 aldy505 commented Jun 25, 2023

This is a PR that tackles getsentry/self-hosted#2211 (comment) and getsentry/self-hosted#2211 (comment) on event that people at Sentry don't want to use an external cron scheduler, and wanted to put it as a vroom extension (of sorts).

I don't think I'll have any free time during next two weeks, meaning I'll be late to response. To mitigate that, I'll just create this one and let you guys decide it.

To apply this on self-hosted repo, I (or someone) just need to modify this on the docker-compose.yml file:

  vroom-cleanup:
    <<: *restart_policy
    image: "$VROOM_IMAGE"
    entrypoint: "/bin/cleanup"
    environment:
      SENTRY_BUCKET_PROFILES: file://localhost//var/lib/sentry-profiles
      # Leaving the value empty to just pass whatever is set
      # on the host system (or in the .env file)
      SENTRY_EVENT_RETENTION_DAYS:
    volumes:
      - sentry-vroom:/var/lib/sentry-profiles

Closes #268

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Copy link
Contributor

@phacops phacops left a comment

Choose a reason for hiding this comment

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

I'm not sure why we'd need a dedicated binary to do that when this command will do the same thing and be implemented nicely in self-hosted:

find ${SENTRY_BUCKET_PROFILES} -type f -mtime +${SENTRY_EVENT_RETENTION_DAYS} -delete

For now, I don't think we should merge this.

@@ -6,6 +6,8 @@ COPY . .

RUN GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -o . -ldflags="-s -w -X main.release=$(git rev-parse HEAD)" ./cmd/vroom

RUN GOOS=linux GOARCH=amd64 CGO_ENABLED=0 go build -o . -ldflags="-s -w" ./cmd/cleanup
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this on another line, we can add ./cmd/cleanup to the line above.

@@ -18,6 +20,8 @@ ENV SENTRY_BUCKET_PROFILES=file://localhost/$PROFILES_DIR

COPY --from=builder /src/vroom /bin/vroom

COPY --from=builder /src/cleanup /bin/cleanup
Copy link
Contributor

Choose a reason for hiding this comment

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

We should change the COPY above to include /src/cleanup instead.

@aldy505
Copy link
Contributor Author

aldy505 commented Jul 6, 2023

@aldy505
Copy link
Contributor Author

aldy505 commented Jul 17, 2023

@phacops regarding getsentry/self-hosted#2211 (comment), can you please close this PR when you've migrated vroom to Debian? Or should I also do the base image migration?

@aldy505
Copy link
Contributor Author

aldy505 commented Jul 18, 2023

Closing in favor of #290

@aldy505 aldy505 closed this Jul 18, 2023
@aldy505 aldy505 deleted the ref/cleanup-binary branch July 18, 2023 14:06
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.

Create a cleanup job based on specified retention days
2 participants