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 stats for query and cache requests #65

Merged
merged 4 commits into from Sep 7, 2022

Conversation

GKosheev
Copy link
Contributor

@GKosheev GKosheev commented Aug 6, 2022

Issue #64

I implemented get method to get the size of cached calls:

get numberOfCachedCalls() {
    return this._cache.size
}

@GKosheev GKosheev changed the title Added stats to check number of cached calls Added get method to check number of cached calls Aug 6, 2022
@szmarczak
Copy link
Owner

This is actually incorrect. It doesn't return the total number of cached requests, but the current cache size. I'd implement it like this:

this.stats = {
	cache: 0,
	query: 0,
};

and increment the counters respectively in

async query(hostname) {

You also need to provide some tests.

@szmarczak
Copy link
Owner

@GKosheev No need to duplicate code in the comments :) There's a review feature and you can add comments there if necessary.

source/index.js Outdated Show resolved Hide resolved
source/index.js Outdated Show resolved Hide resolved
source/index.js Show resolved Hide resolved
source/index.js Show resolved Hide resolved
tests/test.js Outdated Show resolved Hide resolved
@GKosheev
Copy link
Contributor Author

I'm not sure if I did the unit test as you wanted me to, I tried to follow the example you sent me with the verify method.
But I am always ready to make corrections.

@GKosheev
Copy link
Contributor Author

@szmarczak, should I change something specific in the last commit?

tests/test.js Outdated Show resolved Hide resolved
tests/test.js Outdated Show resolved Hide resolved
@GKosheev
Copy link
Contributor Author

GKosheev commented Sep 7, 2022

@szmarczak, would that work?

@szmarczak
Copy link
Owner

Will take a look now, sorry for delay. Also, no need to post a comment of your commit changes :)

@szmarczak szmarczak changed the title Added get method to check number of cached calls Add stats for query and cache requests Sep 7, 2022
@szmarczak szmarczak merged commit ab8d286 into szmarczak:master Sep 7, 2022
@szmarczak
Copy link
Owner

Looks good! Thanks for contributing! ❤️

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.

None yet

2 participants