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

add support for entity type filtering on endpoint search #1055

Conversation

pjhinton-globus
Copy link
Contributor

Add new command line option to globus endpoint search which limits the returned values to certain types of collections. Make use of change to Python SDK work in 37389.

refs sc-29242

@pjhinton-globus pjhinton-globus force-pushed the pjhinton/sc-29242-endpoint-search-filter-entity-type branch 2 times, most recently from aa12186 to 0a71c12 Compare December 3, 2024 22:58
@pjhinton-globus
Copy link
Contributor Author

Once the Globus SDK version in pyproject.toml moves to version 3.49.0 or later, this pull request will move to full open status.

@pjhinton-globus pjhinton-globus force-pushed the pjhinton/sc-29242-endpoint-search-filter-entity-type branch from 0a71c12 to cfa3ccf Compare December 11, 2024 17:44
@pjhinton-globus
Copy link
Contributor Author

Once the Globus SDK version in pyproject.toml moves to version 3.49.0 or later, this pull request will move to full open status.

It appears that the version of the Globus SDK in pyproject.toml has been bumped accordingly by dependabot. Will convert to full PR for review.

@pjhinton-globus pjhinton-globus marked this pull request as ready for review December 11, 2024 17:48
@pjhinton-globus pjhinton-globus force-pushed the pjhinton/sc-29242-endpoint-search-filter-entity-type branch 2 times, most recently from ffdd589 to a7a61e5 Compare January 7, 2025 15:18
Copy link
Member

@sirosen sirosen left a comment

Choose a reason for hiding this comment

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

Initially, I was going to recommend that this change include a new test, but I now see that we don't have tests which exercise this command.
The ideal would be that there are already tests for endpoint search which demonstrate how to test this, and I could point you at an appropriate one to work off of as a template. But we're missing that bit of framework, and I don't think it's fair to ask you to set that up as part of a first contribution.

I'm taking testing as a follow-up item. I have a specific plan for how to test such an addition, and I'll @-mention you on that when I have it up, so you can see what a test would look like.


Can you give this a quick rebase to get passing CI, and optionally apply my suggestion to put backticks in place? That will put this in a clean-to-merge state.

@@ -0,0 +1,3 @@
### Enhancements

* Added a --filter-entity-type option on endpoint search.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Added a --filter-entity-type option on endpoint search.
* Added a `--filter-entity-type` option on endpoint search.

Small tweak. Since this renders on the doc site it's nice to give code-style.

pjhinton-globus added a commit to pjhinton-globus/globus-cli that referenced this pull request Jan 9, 2025
Implements requested change from @sirosen, adding
formatting to the command line option cited in
the changelog fragment.

globus#1055 (comment)
Add new command line option to globus endpoint search which
limits the returned values to certain types of collections.
Make use of change to Python SDK work in 37389.

refs sc-29242

add formatting for option in changelog

Implements requested change from @sirosen, adding
formatting to the command line option cited in
the changelog fragment.

globus#1055 (comment)
@pjhinton-globus pjhinton-globus force-pushed the pjhinton/sc-29242-endpoint-search-filter-entity-type branch from b066a0c to 63d104a Compare January 9, 2025 15:51
@pjhinton-globus
Copy link
Contributor Author

@sirosen wrote:

Can you give this a quick rebase to get passing CI, and optionally apply my suggestion to put backticks in place? That will put this in a clean-to-merge state.

Done! Let me know if any other adjustments need to be made. Thanks again!

@sirosen sirosen merged commit bce3d54 into globus:main Jan 9, 2025
6 checks passed
@pjhinton-globus pjhinton-globus deleted the pjhinton/sc-29242-endpoint-search-filter-entity-type branch January 9, 2025 17:49
sirosen pushed a commit to sirosen/globus-cli that referenced this pull request Jan 13, 2025
Add new command line option to globus endpoint search which
limits the returned values to certain types of collections.
Make use of change to Python SDK work in 37389.

refs sc-29242

add formatting for option in changelog

Implements requested change from @sirosen, adding
formatting to the command line option cited in
the changelog fragment.

globus#1055 (comment)
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.

2 participants