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

Remove List{Domains,Hosts} commands #2545

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

weiminyu
Copy link
Collaborator

@weiminyu weiminyu commented Sep 5, 2024

These commands have been broken for a long time due to result sizes.

We can enhance them by pagination. However, since no one has requested a fix, removal makes more sense.


This change is Reviewable

@weiminyu weiminyu requested a review from CydeWeys September 5, 2024 16:10
Copy link
Member

@CydeWeys CydeWeys 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 following the logic here -- these commands seem to work for me, at least when you don't specify a query that returns too many results? Maybe we should simply change the max results default from MAX_INT to 1000? The queries themselves seem performant enough.

Reviewable status: 0 of 11 files reviewed, all discussions resolved

Copy link
Member

@CydeWeys CydeWeys left a comment

Choose a reason for hiding this comment

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

(And for the record, I do use nomulus list_domains on occasion, e.g. to see how many domains we're up to on our .brand TLDs.)

Reviewable status: 0 of 11 files reviewed, all discussions resolved

These commands have been broken for a long time due to result sizes.

We can enhance them by pagination. However, since no one has requested
a fix, removal makes more sense.
@weiminyu weiminyu force-pushed the remove-list-commands branch from 2e3a8ea to 272b517 Compare September 9, 2024 18:58
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