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

[SVCS-475] Support to log metadata GET requests #277

Conversation

AddisonSchiller
Copy link
Contributor

refs: https://openscience.atlassian.net/browse/SVCS-475

Purpose

Initial support for logging GET requests

Summary of changes

Removed line stopping metadata GET requests from being logged.
Added set up for logging metadata
Added metadata to the callback ignore list. This can be removed once OSF has the correct support for the new logging feature

removed a superfluous return

Test/QA notes.

https://github.com/CenterForOpenScience/osf.io/blob/0f184000e7517f030a8b920818461279364109bd/addons/base/views.py#L299-L307

Here is where I'm pretty sure the OSF is defining what logs it wants. Since metadata does not exist here, if it isn't ignored it throws a lot of annoying error logs/messages in the waterbutler container.

Removed superfluous return in remote_logging
Metadata requests are currently not logged
as they need an OSF addition to work correctly.
@coveralls
Copy link

coveralls commented Oct 4, 2017

Coverage Status

Coverage increased (+0.05%) to 85.401% when pulling 4d68cfb on AddisonSchiller:feature/metadata-logging into 0411fa9 on CenterForOpenScience:develop.

@AddisonSchiller AddisonSchiller changed the title [File Provenance] Support to log metadata GET requests [SVCS-475] Support to log metadata GET requests Oct 4, 2017
@Johnetordoff
Copy link
Contributor

There should be tests for this, I know it there probably none to modify, but whipping up a test to ensure things are logged on_finish is critical. It's kind of a "Who watches the watchman?", situation with logs because use them to debug if there wrong we probably will find out in prod without unittests.

Copy link
Contributor

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

@Johnetordoff Can you take this ticket since you already commented on it? Thanks!

  • Add/update tests (if necessary)
  • Add a # TODO in the code and/or a note in the PR/Ticket indicating what/Where needs to be updated for this to take effect.
  • Check with DjangoLE team to see if this is what they want and they know what to do with OSF side.

Finally, you can create a new PR referencing this one if you made any further changes. Please use rebase so we can keep the ownership of the original commits. Here is what I usually do:

hub checkout https://github.com/CenterForOpenScience/waterbutler/pull/277/
git checkout -b feature/metadata-logging
git fetch upstream develop
git rebase upstream/develop

@coveralls
Copy link

coveralls commented Dec 11, 2017

Coverage Status

Coverage increased (+0.05%) to 89.831% when pulling b9dd890 on AddisonSchiller:feature/metadata-logging into 6249a7a on CenterForOpenScience:develop.

@cslzchen
Copy link
Contributor

@Johnetordoff ignore my comments, just talked with @felliott and we will leave ticket assignment open for now.

@cslzchen
Copy link
Contributor

cslzchen commented Jan 3, 2018

PR closed. Work continues in #307.

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.

4 participants