-
Notifications
You must be signed in to change notification settings - Fork 3
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 CMR and access metrics #58
base: main
Are you sure you want to change the base?
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
I think after we merge this, we should immediately remove the CMR queries since MAAP CMR is going away. But I'm going to share those with the CMR team. |
@@ -0,0 +1,1062 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
client references a global variable without passing to the function or being part of a class
Reply via ReviewNB
@@ -0,0 +1,1062 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line #42. if len(results_list) == no_results:
Can you comment what this no_results does, not particularly clear from the code.
Reply via ReviewNB
@@ -0,0 +1,1062 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line #52. # Boreal TIF files were not stored in sub-directory
Should we fix that? Ticket elsewhere...
Reply via ReviewNB
@@ -0,0 +1,1062 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think 2.0 version of this should cache the results, so the query can optionally be rerun, but doesn't necessarily need to be if the data's already been pulled. (Not a blocker for the PR)
Reply via ReviewNB
@@ -0,0 +1,304 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make a markdown section that explains what the purpose of this notebook is?
"Analyze CMR logs for Collection search using Collection Concept ID, ie finding a collection"
Reply via ReviewNB
@@ -0,0 +1,300 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make a markdown section that explains what the purpose of this notebook is?
"Analyze CMR logs of collection granules(items)"
Reply via ReviewNB
@@ -0,0 +1,359 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make a markdown section that explains what the purpose of this notebook is?
"Analyze CMR logs for use of attribute search parameters by users"
Reply via ReviewNB
@@ -0,0 +1,405 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you make a markdown section that explains what the purpose of this notebook is?
"Analyze CMR logs for Collection search using Collection Short Name, ie finding a collection"
Reply via ReviewNB
Thanks for reviewing @wildintellect - just noting that I plan to address these comments late this week or next week |
This PR adds a notebook for generating s3 access metrics + plots using Athena and other notebooks for generating metrics for CMR queries.