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

Add tests against local Blazegraph #49

Merged
merged 19 commits into from
Mar 26, 2023

Conversation

vemonet
Copy link
Contributor

@vemonet vemonet commented Mar 23, 2023

This PR adds tests to check if a local instance of blazegraph is running, and if so, see if federation works against it. If a local instance of Blazegraph isn't running, these tests are skipped.

There was also the idea that the Python code should take care of downloading the Blazegraph jar and running it in the background, but this proved to be pretty tough, so it's been tabled for this PR. here are some notes on what was tried:

def _start_blazegraph(path):
    import subprocess

    proc_blazegraph = Process(
        target=subprocess.run,
        args=([f"java -jar {path}"]),
        kwargs={
            "shell": True,
        },
        daemon=True,
    )
    proc_blazegraph.start()
    time.sleep(1)
    return proc_blazegraph

    # TODO: blazegraph not actually killed, it will continue to run afterward
    # Not sure how to make sure a java subprocess running as daemon is killed
    # self.proc_blazegraph.kill()

    # manual instructions for cancelling process:
    # 1. shell: ps aux | grep blaze
    # 2. find the number of the process
    # 3. shell: kill #

README.md Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Mar 26, 2023

Codecov Report

Merging #49 (ff1afe9) into main (da58e65) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              main       #49   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          479       479           
  Branches       103       103           
=========================================
  Hits           479       479           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@cthoyt
Copy link
Member

cthoyt commented Mar 26, 2023

@vemonet I've now simplified this down to run only if there's already a local blazegraph available - this takes the burden of writing code to run blazegraph out of the code itself (for now). I saved some of the notes on how to potentially update this in this PR's description

@cthoyt cthoyt changed the title Add tests for federated query against public curie endpoint Add tests against local Blazegraph Mar 26, 2023
@cthoyt cthoyt merged commit 74fdc4e into biopragmatics:main Mar 26, 2023
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.

3 participants