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

GP2-1398_comtrade_data_copy #838

Merged
merged 11 commits into from Feb 24, 2021
Merged

Conversation

bobmeredith
Copy link
Contributor

@bobmeredith bobmeredith commented Feb 19, 2021

Moves a copy of filtered comtrade data into the directory-api database.
The new request to get comtrade data supports multiple countries, single commodity code and does no processing, so all the data are returned for the FE to compute changes and trends.
The country list is always a list of iso2 codes.
There is also a new request for country data that allows multiple country's data to be requested in one go, again using iso3. The model names required can be passed in so this can become a generic request to get any data for a country.

A compacted comtrade data extract is in S3. run make manage import_comtrade_data to load it.
This process can take an hour or more I'm afraid.

The extract is generated as follows:
Comtrade data are downloaded as vast csv files (about 6GB), one for each year. There is a management command
make manage import_comtrade_data -- --raw <filename>
That reads through such files and loads your db with only the data that are needed.
Then, a sql dump of the table to generate the single data file that gets uploaded to S3.

  • Change has a jira ticket that has the correct status.
  • Changelog entry added.

@bobmeredith bobmeredith marked this pull request as draft February 19, 2021 09:35
@codecov
Copy link

codecov bot commented Feb 22, 2021

Codecov Report

Merging #838 (97c594c) into develop (46e8ab4) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #838      +/-   ##
===========================================
+ Coverage    97.35%   97.43%   +0.08%     
===========================================
  Files          112      113       +1     
  Lines         3818     3942     +124     
===========================================
+ Hits          3717     3841     +124     
  Misses         101      101              
Impacted Files Coverage Δ
conf/urls.py 100.00% <ø> (ø)
dataservices/helpers.py 100.00% <100.00%> (ø)
...rvices/management/commands/import_comtrade_data.py 100.00% <100.00%> (ø)
dataservices/models.py 100.00% <100.00%> (ø)
dataservices/serializers.py 100.00% <100.00%> (ø)
dataservices/views.py 97.17% <100.00%> (+0.40%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46e8ab4...97c594c. Read the comment docs.

@bobmeredith bobmeredith marked this pull request as ready for review February 23, 2021 15:58
CHANGELOG.md Outdated
@@ -22,7 +23,7 @@
- GP2-1382 - getting paid structure
- GP2-1180 - travel bus BE
- NOTICKET - Exportplan make dict default for JSON fields.
- GP2-1181 - business risk
- GP2-1181 - 1398business risk
Copy link
Contributor

Choose a reason for hiding this comment

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

look like a typo here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the spot.

Copy link
Contributor

@depsiatwal depsiatwal left a comment

Choose a reason for hiding this comment

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

Small comment just on CL
nicely done :-)

trade_value=91.97,
)
yield
models.InternetUsage.objects.all().delete()
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you need to delete all records from ComtradeReport here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hehe - well spotted - thanks

year_2019=13,
)
yield
models.InternetUsage.objects.all().delete()
Copy link
Contributor

Choose a reason for hiding this comment

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

similarly here EaseOfDoingBusiness

Copy link
Contributor

@webbyfox webbyfox left a comment

Choose a reason for hiding this comment

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

minor comments

Copy link
Contributor

@webbyfox webbyfox left a comment

Choose a reason for hiding this comment

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

LGTM

@bobmeredith bobmeredith merged commit ef8bacc into develop Feb 24, 2021
@bobmeredith bobmeredith deleted the GP2-1398_comtrade_data_copy branch February 24, 2021 09:48
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

3 participants