Skip to content

Add missing lib script to sentry-admin.sh #3693

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

Merged
merged 6 commits into from
May 19, 2025
Merged

Conversation

djakielski
Copy link
Contributor

@djakielski djakielski commented May 6, 2025

Sentry Admin Script always fail because of missing import of lib script.

Fixes #3700.

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
Member

@BYK BYK left a comment

Choose a reason for hiding this comment

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

Heya, thanks for the PR! That said I don't think _lib.sh is needed in this script. I haven't seen anything referring to that and we actually test the backup and restore functionality using this script.

Can you share what kind of an error you are encountering for us to understand this better?

@djakielski
Copy link
Contributor Author

Hey @BYK i got Issues that function vergte is unknow when ever I execute sentry-admin.sh no matter which parameter i add. After debugging I found that install/dc-detect-version.sh need this function. After fixing this locally, the script runs normaly.

@BYK
Copy link
Member

BYK commented May 7, 2025

Ah, right sorry! Yeah, since that vergte call is in a conditional I guess we never hit it in CI. Thanks so much for the patch and explanation again!

@BYK BYK enabled auto-merge (squash) May 7, 2025 10:13
@BYK BYK disabled auto-merge May 7, 2025 10:13
@BYK BYK enabled auto-merge (squash) May 7, 2025 10:14
@aminvakil
Copy link
Collaborator

What do you say about moving vergte function to another file and only including that?

I'm seeing this in _lib.sh and I don't think it's appropriate to be executed at running sentry-admin.

log_file=sentry_install_log-$(date +'%Y-%m-%d_%H-%M-%S').txt
exec &> >(tee -a "$log_file")

There may be another problems as well.

@BYK
Copy link
Member

BYK commented May 17, 2025

@aminvakil yeah that makes total sense and probably explains the failure.

I'll give this a go today

@BYK BYK merged commit 031a1f1 into getsentry:master May 19, 2025
5 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jun 4, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

sentry-admin.sh fails to detect latest docker compose version
3 participants