Closed Golub-Sergey closed 2 years ago
Thanks for the pull request, @Golub-Sergey! I've created BLENDED-605 to keep track of it in Jira. More details are on the BD-19 project page.
When this pull request is ready, tag your edX technical lead.
Merging #835 (be6220b) into master (1f9144a) will increase coverage by
0.04%
. The diff coverage is88.52%
.
@@ Coverage Diff @@
## master #835 +/- ##
==========================================
+ Coverage 74.01% 74.05% +0.04%
==========================================
Files 208 206 -2
Lines 23860 23716 -144
==========================================
- Hits 17660 17564 -96
+ Misses 6200 6152 -48
Impacted Files | Coverage Δ | |
---|---|---|
edx/analytics/tasks/insights/module_engagement.py | 79.05% <ø> (-0.09%) |
:arrow_down: |
...ics/tasks/insights/tests/test_module_engagement.py | 99.71% <ø> (-0.01%) |
:arrow_down: |
...tests/acceptance/services/elasticsearch_service.py | 65.00% <50.00%> (+37.72%) |
:arrow_up: |
edx/analytics/tasks/common/elasticsearch_load.py | 88.75% <100.00%> (+3.63%) |
:arrow_up: |
...tics/tasks/common/tests/test_elasticsearch_load.py | 100.00% <100.00%> (ø) |
|
...icsearch_integration/test_elasticsearch_service.py | 100.00% <100.00%> (ø) |
|
...alytics/tasks/util/aws_elasticsearch_connection.py | 100.00% <100.00%> (ø) |
|
edx/analytics/tasks/util/elasticsearch_target.py | 75.00% <100.00%> (+2.02%) |
:arrow_up: |
edx/analytics/tasks/util/record.py | 79.12% <100.00%> (-0.10%) |
:arrow_down: |
edx/analytics/tasks/util/tests/test_record.py | 100.00% <100.00%> (ø) |
|
... and 12 more |
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 1f9144a...5785553. Read the comment docs.
In order to run acceptance tests against this, I think we will need to upgrade the test instance of ES we use. I will work with my team to plan for this work.
In order to run acceptance tests against this, I think we will need to upgrade the test instance of ES we use. I will work with my team to plan for this work.
@estute I'm going to write tests on pieces for code which decrease coverage
I tried to run the acceptance tests tonight, and they failed with the following stacktrace:
File "/usr/local/lib/python2.7.12/lib/python2.7/unittest/case.py", line 320, in run
self.setUp()
File "/var/lib/jenkins/jobs/acceptance-es-upgrade/workspace/analytics-tasks/edx/analytics/tasks/tests/acceptance/__init__.py", line 304, in setUp
self.reset_external_state()
File "/var/lib/jenkins/jobs/acceptance-es-upgrade/workspace/analytics-tasks/edx/analytics/tasks/tests/acceptance/__init__.py", line 329, in reset_external_state
self.elasticsearch.reset()
File "/var/lib/jenkins/jobs/acceptance-es-upgrade/workspace/analytics-tasks/edx/analytics/tasks/tests/acceptance/services/elasticsearch_service.py", line 34, in reset
response = self._elasticsearch_client.indices.get_aliases(name=self._alias)
'\'IndicesClient\' object has no attribute \'get_aliases
I believe the ES API has changed the aliases endpoint. See: https://discuss.elastic.co/t/get-aliases-removed-from-elasticsearch-py-python/76092
pipeline has been moved on to ES7 since this PR so its work is done
@Golub-Sergey Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.
Analytics Pipeline Pull Request
Description: updates ES lib requirements and interfaces which perform tasks that will put data into ES
Notes: Analytics Pipeline to ES7 edx doc.
Was done:
Code coverage: 74%