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

fix: case-insensitive curator comment + fix logger.warning statement #23

Merged
merged 8 commits into from
Aug 10, 2023

Conversation

seasidesparrow
Copy link
Member

modified:   adsdocmatch/oracle_util.py

Fixes Issue #22

 	modified:   adsdocmatch/oracle_util.py
 	modified:   adsdocmatch/oracle_util.py
 	modified:   run.py
 	modified:   adsdocmatch/tests/unittests/test_oracle_util.py
 	modified:   adsdocmatch/oracle_util.py
 	modified:   adsdocmatch/tests/unittests/test_oracle_util.py
 	modified:   run.py
@seasidesparrow
Copy link
Member Author

This PR also adds a method to access the /cleanup endpoint of oracle_service, along with unit tests.

@@ -188,6 +195,12 @@ def main():
else:
logger.error("Both parameters are need to add matched bibcodes to oracle: file path (-mf) and the source to apply (-as).")

elif args.cleanup_oracle:
try:
status = OracleUtil().cleanup_db()
Copy link
Contributor

Choose a reason for hiding this comment

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

I would get the message returned from oracle and display it. There are three types of cleanup details are here https://github.com/adsabs/oracle_service/blob/master/oraclesrv/views.py#L382

Copy link
Member Author

Choose a reason for hiding this comment

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

I refactored this to return response.text and modified the tests accordingly

Copy link
Contributor

@golnazads golnazads Aug 8, 2023

Choose a reason for hiding this comment

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

No, not quit.

Here is what I would do (lines between double stars)

def cleanup_db(self):
    """

    :param:
    :return:
    """
  
    try:
        sleep_sec = int(config.get('DOCMATCHPIPELINE_API_ORACLE_SERVICE_SLEEP_SEC', 5))
        num_attempts = int(config.get('DOCMATCHPIPELINE_API_ORACLE_SERVICE_ATTEMPTS', 5))
        for i in range(num_attempts):
            response = requests.get(
                url=config.get('DOCMATCHPIPELINE_API_ORACLE_SERVICE_URL', 'http://localhost') + '/cleanup',
                headers={'Authorization': 'Bearer %s' % config.get('DOCMATCHPIPELINE_API_TOKEN', '')})
            status_code = response.status_code
            if status_code == 200:
                logger.info('Got 200 for status_code at attempt # %d' % (i + 1))
                break
            elif status_code in [500, 502, 504]:
                logger.info('Got %d status_code from oracle, waiting %d second and attempt again.' % (
                status_code, num_attempts))
                time.sleep(sleep_sec)
            # any other error, quit
            else:
                logger.info('Got %s status_code from a call to oracle, stopping.' % status_code)
                break
        **json_text = json.loads(response.text)**
        **message = json_text.get('message', '')**
        if status_code == 200:
            **logger.info('Cleanup command issued to oracle database, got response %s.'%message)**
        else:
            **logger.warning('Unable to issue cleanup command to oracle_service, service returned status code %s with message %s on final try' % (status_code, message))**
        **return message**
    except Exception as err:
        logger.error("Error from cleanup_db: %s" % err)
        return "Error from cleanup_db: %s" % err

Copy link
Contributor

Choose a reason for hiding this comment

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

Matt, did not like the field message, just renamed it to details in oracle, I think makes more sense. If you would please make the change here. Also for testing, here is the unittest in oracle if you want to see what messages are brought back from cleanup https://github.com/adsabs/oracle_service/blob/master/oraclesrv/tests/unittests/test_oracle_service.py#L762.

Copy link
Member Author

Choose a reason for hiding this comment

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

These changes have been made, thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Great job. Thank you.

 	modified:   adsdocmatch/oracle_util.py
 	modified:   adsdocmatch/tests/unittests/test_oracle_util.py
 	modified:   run.py
 	modified:   adsdocmatch/oracle_util.py
 	modified:   adsdocmatch/tests/unittests/test_oracle_util.py
@seasidesparrow seasidesparrow merged commit b0f3c1f into adsabs:master Aug 10, 2023
1 check passed
@seasidesparrow seasidesparrow deleted the fix_issue_22.20230724 branch November 2, 2023 18:06
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.

2 participants