docs(id-support): Document that -p and -o arguments accept slugs and IDs

[iamrajjoshi]

With the API changes we made to support ids as well as slugs, we get the added bonus that our CLI can also support ids.

I verified this locally for a few commands and it all seems to be working. This is the first of a couple prs to update the CLI to reflect this new support.

In this pr, i add id support for the events list command. To do this, I defined two new args (which are identical but with a better help message) and wrote tests to test this new feature. The API has already been verified to work with both ids and slugs.

rough spec

[szokeasaurusrex]

I am lacking context on the API changes to enable users to use organization/project IDs rather than slugs that you made @iamrajjoshi (in the future, please link changes you are referring to in the PR so that people unfamiliar with your direct work have context on changes you propose), so please be aware that my review is based on a limited understanding of these changes.

Are you planning to support organization/project IDs for all API endpoints in Sentry, or only some of them? And what endpoints are supported now?

From the changes you are proposing, it would seem that only the endpoint hit by the event list command is supported; however, in my testing, I noticed that the issues list, files upload, and releases upload commands can all also take org/project IDs, even though this change does not address the issues list command. Given this PR in the Sentry repo, which claims to update all endpoints to support org/project IDs, it would seem reasonable to infer that all Sentry APIs (and hence, all Sentry CLI commands) should now support org/project IDs; however, this is not the case, since calling sourcemaps upload with a project ID instead of a project slug results in the following error:

error: API request failed
  caused by: sentry reported an error: One or more projects are invalid (http status: 400)

All of this to say, I am very confused about why you are proposing to add so much complexity to the Sentry CLI code in the form of two new functions that are almost identical to existing functions, only so that one of the many commands that support IDs for org/projects have this stated in their help text.

The simplest solution here would be to wait until Sentry supports org/project IDs for all APIs¹ (or at least, all APIs that the CLI accesses), and then simply update the existing org_arg and project_arg functions to indicate that all commands that take these arguments support a slug or ID.

So, let's please wait until all Sentry endpoints actually support org/project endpoints, then just update the existing functions with the new help text. Merging this PR as is would only add unnecessary complexity to the CLI code, and since the CLI code is already way to complex as is, I strongly oppose making it even more complex unless there is a very good reason to do so.

There is no downside to delaying the merging of this PR, since it is, in effect, only a documentation change to the help text, and it does not add any real functionality. The CLI already supports project and org IDs for the event list command.

Footnotes

1. If, for some reason, we do not plan to support IDs for all Sentry APIs, I would suggest we make no changes in the CLI since we already support IDs everywhere the Sentry API does, and this can simply remain a hidden feature. It would probably confuse users though if only some commands supported IDs, so I would advocate allowing IDs for all Sentry API endpoints so they can be universally supported in the CLI. ↩

[szokeasaurusrex]

Suggested change

	]
	]

Let's make sure this file ends in a newline

[loewenheim]

You are still using the same value parser as the org argument here. I think the validation itself should be correct, but if you pass an invalid value, it prints "Invalid value for organization. Use the URL slug and not the name!", which is misleading.

[szokeasaurusrex]

With the changes I requested, we would no longer have a separate org_arg_with_id function.

Instead, we would need to update the existing validate_org value parser to state that an organization slug or ID needs to be provided.

[szokeasaurusrex]

Of course, that also (like the rest of the changes) can only be done once all Sentry APIs support org or project IDs

[iamrajjoshi]

Hey @szokeasaurusrex , thanks for the review! You are right, I should have added a bit more context here about the API changes made for folks less familiar (a shipped post is incoming).

Are you planning to support organization/project IDs for all API endpoints in Sentry, or only some of them? And what endpoints are supported now?

We now support Ids and Slugs in the path parameters of all our APIs. This has been tested via tests for all the endpoints. I just looked into the sourcemaps upload command and turns out the endpoint it is hitting (OrganizationArtifactBundleAssembleEndpoint) passes the project slug as a body parameter, something that we haven't updated for ids yet.

My plan isn't to add extra complexity. I wanted to roll out this documentation changes slowly, while testing all the commands to make sure they work with both identifiers. I was planning on updating the help strings in batches as I wrote tests and tested all the commands. I added these additional two functions temporarily until all the commands had been tested, then deleted them.

That being said, if we want to make this change all together, let me find all APIs that the CLI hits and check if there are any other commands that hit endpoints that pass slugs as body params (which i am fairly certain shouldn't be many) and update those endpoints before make a blanket pr to update the docs.

[szokeasaurusrex]

Are we planning to support IDs for body parameters, as well?

• If yes, I would say we should update the help text for all commands simultaneously by changing the existing org/project_arg functions. If this was a new feature (in the CLI), I would agree it makes some sense to roll out to commands on a slower basis, but since here we're only really changing the help text, let's roll out at once to all commands.

• If not, I think the best option would be simply to close this PR and keep the help text as is for all commands. I am assuming that the org and project slugs will continue to work, and so that should continue to be what we recommend to users since it is the only option that works for all commands. Users who really want to use IDs for whatever reason can do so, even without the help text stating this is possible, for the APIs that support IDs, since IDs pass the CLI's validation for slugs already.

Please let me know if you have any questions!

[iamrajjoshi]

Are we planning to support IDs for body parameters, as well?

I am planning to add support for api body params that are used by the CLI as of now. Then I'll just change the help strings in bulk.

After I rename, if someone is adding functionality to CLI then they will know that they should consider id as well and create the support for API if needed.

[iamrajjoshi]

Closing this PR for now, will make another one to change the help string sin bulk.