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
1 change: 1 addition & 0 deletions adsdocmatch/oracle_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -573,6 +573,7 @@ def cleanup_db(self):
"""

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(
Expand Down
13 changes: 13 additions & 0 deletions run.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,13 @@ def get_args():
default=False,
help="Add tab delimited two column matched bibcodes to oracle with confidence of the source.")

parser.add_argument("-c",
"--cleanup",
dest="cleanup_oracle",
action="store_true",
default=False,
help="Clean up the db, removing tmp bibcodes and lower confidence of multi matches.")

return parser.parse_args()


Expand Down Expand Up @@ -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.

logger.info("The cleanup_db command was issued to oracle_service.")
except Exception as err:
logger.error("Error issuing cleanup_db command to oracle_service: %s" % err)
else:
logger.debug("Nothing to do.")

Expand Down
Loading