Iqss/#8380 counter-processor version update #8391
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pdurbin
reviewed
Mar 14, 2022
| @@ -425,20 +425,30 @@ Counter Processor has only been tested on el7 (see "Linux" above). Please note t | |||
| As root, download and install Counter Processor:: | |||
|
|
|||
| cd /usr/local | |||
| wget https://github.com/CDLUC3/counter-processor/archive/v0.0.1.tar.gz | |||
| wget https://github.com/CDLUC3/counter-processor/archive/v0.1.04.tar.gz | |||
| tar xvfz v0.0.1.tar.gz | |||
There was a problem hiding this comment.
This tar still shows the old version.
|
|
||
| As root, change to the Counter Processor directory you just created, download the GeoLite2-Country tarball, untar it, and copy the geoip database into place:: | ||
| Counter Processor can report per country results if the optional GeoLite Country Database is installed. At present, this database is free but to use it one nust signing an agreement (EULA) with MaxMind. |
| @@ -83,9 +83,9 @@ Configure Counter Processor | |||
|
|
|||
There was a problem hiding this comment.
Just a general comment I'm adding to the top... the branch has a # in it. I forget if this causes problems deploying to QA servers or not.
There was a problem hiding this comment.
FWIW: it is easy to rename the branch but I think that closes the PR (any way around that?) - let me know if this is needed for the QA build.
|
|
||
| As root, change to the Counter Processor directory you just created, download the GeoLite2-Country tarball, untar it, and copy the geoip database into place:: | ||
| Counter Processor can report per country results if the optional GeoLite Country Database is installed. At present, this database is free but to use it one nust signing an agreement (EULA) with MaxMind. | ||
| (The primary concern appears to be that individuals can opt-out of having their location tracked via IP address and, due to various privacy laws, MaxMind needs a way to comply with that for products it has 'sold' (for no cost in this case). Their agreement requires you to either configure automatic updates to the GeoLite Country database or be responsible on your own for managing take down notices.) |
There was a problem hiding this comment.
I'd put "sold" in double quotes instead of single quotes.
| @@ -11,7 +11,7 @@ python3.6 -m ensurepip | |||
| COUNTER_USER=counter | |||
| echo "Ensuring Unix user '$COUNTER_USER' exists" | |||
| useradd $COUNTER_USER || : | |||
| COMMIT='a73dbced06f0ac2f0d85231e4d9dd4f21bee8487' | |||
| COMMIT='7974dad259465ba196ef639f48dea007cae8f9ac' | |||
There was a problem hiding this comment.
We don't need to change this right now but we could switch to a release instead of commit when downloading. Originally, we used a commit because there were no releases ( CDLUC3/counter-processor#2 ).
pdurbin
reviewed
Mar 14, 2022
pdurbin
approved these changes
Mar 14, 2022
qqmyers
added a commit
to QualitativeDataRepository/dataverse
that referenced
this pull request
Apr 13, 2022
…erprocesor_version_update
|
To make counter-processor 0.1.04 the new default version (e.g. with install instructions for it in the guides, etc.), some additional work is needed on the PR to update the python version (to 3.7+) and update it's install instructions/scripts, etc. |
…://github.com/GlobalDataverseCommunityConsortium/dataverse.git into IQSS/IQSS#8380-counterprocesor_version_update
…sor_version_update IQSS#8390 Counter Processor v0.1.04 requires Python 3.7 or higher.
pdurbin
reviewed
Apr 14, 2022
| Installing GeoLite Country Database | ||
| =================================== | ||
|
|
||
| Counter Processor can report per country results if the optional GeoLite Country Database is installed. At present, this database is free but to use it one must signing an agreement (EULA) with MaxMind. |
There was a problem hiding this comment.
Suggested change
| Counter Processor can report per country results if the optional GeoLite Country Database is installed. At present, this database is free but to use it one must signing an agreement (EULA) with MaxMind. | |
| Counter Processor can report per country results if the optional GeoLite Country Database is installed. At present, this database is free but to use it one must sign an agreement (EULA) with MaxMind. |
qqmyers
added a commit
to QualitativeDataRepository/dataverse
that referenced
this pull request
May 6, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it: This PR updates the parsing of the counter-processor reports in Dataverse to be compatible with the v0.1.04 (latest) version of counter processor and updates the documents to reference that version. It also updates the documentation related to MaxMind's change from making the GeoLite Country database available from a fixed URL to something you have to sign up for and sign an agreement to download. The changes point to MaxMind's documentation about why this was done and how to obtain the database, and notes that this is an optional addition to counter-processor (used to report per country results).
Which issue(s) this PR closes:
Special notes for your reviewer: The vagrant script references the non-working GeoLite db download URL. I think it is currently failing gracefully but this section could perhaps be removed. Since the new process requires having an account/signing a EULA with MaxMind, probably configuring a cron job to run their update script, etc. I'm not sure it is something we could/should automate. So it may make sense to do something like write a message to say 'follow the guides to install the optional GeoLite Country db for counter-processor to capture per country MakeDataCount statistics.' I'm hoping someone might suggest a change for the script.
Suggestions on how to test this: I'll try to test at QDR and report on/fix any issues I find there. For QA testing, configure for Make Data Counts. Update the counter-processor version to v0.1.04 and test that daily results are still being captured. The update should also be backward compatible, i.e. running this PR with counter processor v0.0.1 should also work.
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Additional documentation: