[SVCS-475] Send hook request to OSF for GET requests#307
[SVCS-475] Send hook request to OSF for GET requests#307Johnetordoff wants to merge 7 commits intoCenterForOpenScience:developfrom
Conversation
Removed superfluous return in remote_logging Metadata requests are currently not logged as they need an OSF addition to work correctly.
…ts to track that behavior.
|
|
||
| return payload | ||
|
|
||
| def __eq__(self, other: object) -> bool: |
There was a problem hiding this comment.
👍 I can't think of a better solution than __eq__.
cslzchen
left a comment
cslzchen
left a comment
There was a problem hiding this comment.
I will fix other unrelated test failures due to the __version__ issue as well.
tests/core/test_remote_logging.py
Outdated
| from waterbutler.core.log_payload import LogPayload | ||
| from waterbutler.core.remote_logging import log_to_callback | ||
|
|
||
| from tests.providers.osfstorage.fixtures import ( |
There was a problem hiding this comment.
Fix import style as shown in previous examples.
tests/core/fixtures.py
Outdated
|
|
||
| import pytest | ||
|
|
||
| from tests.utils import MockCoroutine |
There was a problem hiding this comment.
from waterbutler.core.path import WaterButlerPath
from waterbutler.core.log_payload import LogPayload
from tests.utils import MockCoroutine
from tests.providers.osfstorage.fixtures import (auth, file_path, file_lineage, provider,
file_metadata_object, file_metadata)
tests/core/test_remote_logging.py
Outdated
| mock_time): | ||
|
|
||
| with mock.patch('waterbutler.core.utils.send_signed_request', mock_signed_request): | ||
| await log_to_callback('move', log_payload, log_payload) |
There was a problem hiding this comment.
In remote_logging.py, the signature for log_to_callback() is:
async def log_to_callback(action, source=None, destination=None, start_time=None, errors=[]):This doesn't look right. I removed the two No, they failed.log_payload arguments and tests passed as expected.
| @@ -0,0 +1,122 @@ | |||
| import time | |||
There was a problem hiding this comment.
Fix import order and style according to examples I added above.
tests/core/test_remote_logging.py
Outdated
| with pytest.raises(Exception) as exc: | ||
| await log_to_callback('upload', log_payload, log_payload) | ||
| assert exc.message == 'Callback for upload request failed with {},' \ | ||
| ' got {"status": "failure"}'.format(MockBadResponse()) |
There was a problem hiding this comment.
Curly braces are escaped by using {{ and }} in python.
tests/server/api/v1/fixtures.py
Outdated
|
|
||
| @pytest.fixture | ||
| def source_payload(handler): | ||
| return LogPayload(handler.resource, |
There was a problem hiding this comment.
Fix style for arguments in function/method call.
|
|
[SVCS-475] Fix minor bugs and improve style
5 participants
Ticket
https://openscience.atlassian.net/browse/SVCS-475
Purpose
This PR replaces: #277.
The goal of this PR is to send a hook request for all file requests, including GETs so that the osf-side can better track provider metadata. I've also added some unit tests to improve coverage going forward.
Changes
Modifies the accepted actions for sending hooks and creates groups of parameterized tests so the desired action behavior is more obvious.
Side effects
In order to compare LogPayload instances which aren't returned by the logging function, I've have to modify the eq method of the the LogPayload to compare by serialization instead of the typical instance to instance comparison.
QA Notes
None. This is not a user facing change.
Deployment Notes
Requires osf-side changes to have any utility.