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

Some tweaks for stats and top-clients #124

Merged
merged 4 commits into from
Aug 13, 2017

Conversation

DL6ER
Copy link
Member

@DL6ER DL6ER commented Aug 13, 2017

By submitting this pull request, I confirm the following (please check boxes, eg [X]) Failure to fill the template will close your PR:

Please submit all pull requests against the development branch. Failure to do so will delay or deny your request

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

How familiar are you with the codebase?:

10


Currently, unique_clients (in the output of >stats) returns the number of clients that FTL has ever seen in its runtime. However, this is inconsistent with the data returned by >top-clients which only contains clients that are active, i.e. have made at least one query in the most recent 24 hours.

This PR makes the behavior more consistent:

  • >stats will return unique_clients that is consistent with the >top-lists output (count only active clients). We add a new property clients_ever_seen to preserve the information of how many unique clients FTL has seen in its runtime (whether or not they have been active recently) to not loose the availability of this data.

  • We add withzero as an option to >top-clients (e.g. >top-clients withzero (15)). With this flag enabled, the top clients result will contain all clients that are know to FTL, i.e. also those which haven't been active recently.

  • Update README.md

This template was created based on the work of udemy-dl.

DL6ER added 3 commits August 13, 2017 14:52
…nts that have zero queries within the most recent 24 hours
…active within the most recent 24 hours (match the behavior of top-clients). Add "clients_ever_seen" to preserve the information of how many unique clients FTL has seen in its runtime (whether or not they have been active recently)
@DL6ER DL6ER requested a review from WaLLy3K August 13, 2017 13:16
Copy link

@WaLLy3K WaLLy3K left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@WaLLy3K WaLLy3K left a comment

Choose a reason for hiding this comment

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

Unsurprisingly, still approve worthy even with added comments.

@DL6ER DL6ER requested review from dschaper and AzureMarker August 13, 2017 13:23
@DL6ER
Copy link
Member Author

DL6ER commented Aug 13, 2017

Note that these changes will propagate through to the PHP API so there is no need for changes on the web repo.

@AzureMarker AzureMarker merged commit d5d8f47 into development Aug 13, 2017
@AzureMarker AzureMarker deleted the tweak/top-clients-withzero branch August 13, 2017 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants