-
-
Notifications
You must be signed in to change notification settings - Fork 239
feat(preprod): Add snapshots subcommand #3110
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
base: master
Are you sure you want to change the base?
Conversation
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Features
- Add snapshots subcommand ([#3110](https://github.com/getsentry/sentry-cli/pull/3110))If none of the above apply, you can opt out of this check by adding |
| #[derive(Deserialize, Debug)] | ||
| pub struct OrganizationDetails { | ||
| pub id: String, | ||
| pub links: OrganizationLinks, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This endpoint has a bunch more information but what we really care about is the id and links.
The region_url contains the regional API url, e.g. us.sentry.io. This way we can hit that endpoint directly instead of going through control silo (sentry.io).
If I understand correctly I think this information might already be somehow embedded in org tokens, but not in personal tokens. Or maybe I'm confusing this with something else.
| use anyhow::{Context, Result}; | ||
|
|
||
| /// Given an org and project slugs or IDs, returns the IDs of both. | ||
| pub fn get_org_project_id(api: impl AsRef<Api>, org: &str, project: &str) -> Result<(u64, u64)> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As far as I know, for all commands a user can provide --project and --org as slugs or IDs.
These utils are needed to get the corresponding IDs, so that objects from the same org/proj all have the same paths in objectstore regardless if the user passed in slugs or IDs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be weird to take in Api and it might be possible that these functions should live somewhere else, IDK.
They certainly don't belong to the Api struct as its methods seem to all map 1to1 to Sentry API calls.
| lazy_static = "1.4.0" | ||
| libc = "0.2.139" | ||
| log = { version = "0.4.17", features = ["std"] } | ||
| objectstore-client = { git = "https://github.com/getsentry/objectstore.git", branch = "lcian/feat/rust-batch-client" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This indirectly adds a dependency to reqwest and we need to double check what the implications of that are, especially in regard to rustls vs native-tls which could be problematic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also adds a bunch of deps but I think most of them are inevitable.
Updated version of #3049 to discuss and iterate on things.
Notable changes:
shard_indexparameter from the command. I'm not sure what the purpose of that was originally.many(batch) API fromobjectstore_client. All uploads are executed as batch requests, reducing network overhead. Unfortunately, with they way things are implemented now, we will still have to buffer all files in memory before sending the request, as we need to hash their contents to determine the filename. If we could just use the filename as the key in objectstore, it would be much better because that way we could stream the files over.Note that auth enforcement still needs to be enabled for objectstore, so that's currently blocking this to be used for anything but internal testing.
Ref FS-233