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

Reformat codebase #137

Open
wants to merge 30 commits into
base: add_pydantic_support
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
246e07a
Refactor linter
nuwang Jul 26, 2024
e906d82
Initial use of pydantic for entity validation
nuwang Aug 8, 2024
b1b2325
Add black configuration
nuwang Aug 12, 2024
d9922b6
Fix invalid tag tests
nuwang Aug 12, 2024
c46641b
Fix loader propagation and other minor fixes
nuwang Aug 12, 2024
43a7915
Gpus can only be whole numbers
nuwang Aug 12, 2024
63ecdea
Process inheritance of loaded entities
nuwang Aug 12, 2024
3f06eda
Only apply Rule properties if instance of Rule
nuwang Aug 12, 2024
5622021
Change factory for GlobalConfig
nuwang Aug 12, 2024
ab6ffac
Allow cores and mem to be int as well
nuwang Aug 12, 2024
359386a
Fix bug in setting tags
nuwang Aug 12, 2024
c800655
Fix incorrect variable reference
nuwang Aug 12, 2024
9fba359
Change field ordering
nuwang Aug 12, 2024
8793622
Fix pattern for tag filtering
nuwang Aug 12, 2024
1099131
Add error message when failing to load a file
nuwang Aug 13, 2024
fd81f04
Allow any datatype for params and convert env to str
nuwang Aug 13, 2024
e4d1832
Fix tag_values_match function in helpers
nuwang Aug 13, 2024
503ade6
Add back entity.filter() and fix score function
nuwang Aug 14, 2024
a938add
Make sure min_cores/mem etc. types match specified
nuwang Aug 15, 2024
41c38e3
Restore original tag_values_match helper function
nuwang Aug 15, 2024
0607018
Allow bool or str for if conditions
nuwang Aug 15, 2024
151040d
Fix score function
nuwang Aug 15, 2024
696c875
Fix tests for invalidly tagged users
nuwang Aug 15, 2024
9e0e047
Switch tests to python 3.11
nuwang Aug 15, 2024
791f52d
Refactor code evaluation interface
nuwang Aug 15, 2024
a137cf0
Improve linting and formatting commands with black and isort
nuwang Aug 25, 2024
5ea80d0
Run isort against codebase
nuwang Aug 25, 2024
9759507
reformat codebase using black
nuwang Aug 25, 2024
af57588
Add format command and reformat test code
nuwang Aug 25, 2024
39acaf8
Disable slow tests
nuwang Aug 25, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ jobs:
run: pip install tox

- name: Run tox
run: tox -e py${{ matrix.python-version }} -- --runslow
# run: tox -e py${{ matrix.python-version }} -- --runslow
run: tox -e py${{ matrix.python-version }}
env:
PYTHONUNBUFFERED: "True"

Expand Down
7 changes: 7 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,10 @@ match=^[Tt]est

[bdist_wheel]
universal = 1

[black]
line-length = 88

[isort]
line_length = 88
profile = black
2 changes: 1 addition & 1 deletion tests/fixtures/linter/linter-invalid-regex.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ tools:
cores: 2
params:
native_spec: "--mem {mem} --cores {cores} --gpus {gpus}"
bwa[0-9]++:
bwa[0-9]++\:
gpus: 2

destinations:
Expand Down
6 changes: 0 additions & 6 deletions tests/fixtures/mapping-basic.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ tools:
scheduling:
require:
- non_existent
invalidly_tagged_tool:
scheduling:
require:
- general
reject:
- general
regex_tool.*:
scheduling:
require:
Expand Down
4 changes: 2 additions & 2 deletions tests/fixtures/mapping-destinations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ destinations:
SPECIAL_FLAG: "first"
params:
memory_requests: "{mem}"
k8s_walltime_limit: 10
k8s_walltime_limit: "10"
rules:
- if: input_size > 10
execute: |
Expand All @@ -193,7 +193,7 @@ destinations:
TEST_ENTITY_PRIORITY: "{cores*2}"
params:
memory_requests: "{mem*2}"
k8s_walltime_limit: 20
k8s_walltime_limit: "20"
rules:
- if: input_size > 20
execute: |
Expand Down
6 changes: 0 additions & 6 deletions tests/fixtures/mapping-invalid-regex.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ tools:
scheduling:
require:
- non_existent
invalidly_tagged_tool:
scheduling:
require:
- general
reject:
- general
regex_tool.*:
scheduling:
require:
Expand Down
40 changes: 40 additions & 0 deletions tests/fixtures/mapping-invalid-tags.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
global:
default_inherits: default

tools:
default:
abstract: true
cores: 2
mem: 8
env: {}
scheduling:
require: []
prefer:
- general
accept:
reject:
- pulsar
rules: []
invalidly_tagged_tool:
scheduling:
require:
- general
reject:
- general

destinations:
local:
runner: local
max_accepted_cores: 4
max_accepted_mem: 16
scheduling:
prefer:
- general
k8s_environment:
runner: k8s
max_accepted_cores: 16
max_accepted_mem: 64
max_accepted_gpus: 2
scheduling:
prefer:
- pulsar
6 changes: 0 additions & 6 deletions tests/fixtures/mapping-rank.yml
Original file line number Diff line number Diff line change
Expand Up @@ -46,12 +46,6 @@ users:
scheduling:
require:
- earth
[email protected]:
scheduling:
require:
- pulsar
reject:
- pulsar
.*@vortex.org:
scheduling:
require:
Expand Down
6 changes: 0 additions & 6 deletions tests/fixtures/mapping-role.yml
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,6 @@ users:
scheduling:
require:
- earth
[email protected]:
scheduling:
require:
- pulsar
reject:
- pulsar
.*@vortex.org:
scheduling:
require:
Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/mapping-rule-argument-based.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ tools:
rules:
- if: |
helpers.job_args_match(job, app, {'input_opts': {'db_selector': 'db'}})
id: limbo_rule
scheduling:
prefer:
- pulsar
Expand Down
6 changes: 0 additions & 6 deletions tests/fixtures/mapping-rules-changed.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,6 @@ users:
max_mem: cores * 6
- if: input_size >= 5
fail: Just because
[email protected]:
scheduling:
require:
- pulsar
reject:
- pulsar
.*@vortex.org:
scheduling:
require:
Expand Down
6 changes: 0 additions & 6 deletions tests/fixtures/mapping-rules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,6 @@ users:
max_mem: cores * 6
- if: input_size >= 5
fail: Just because
[email protected]:
scheduling:
require:
- pulsar
reject:
- pulsar
.*@vortex.org:
scheduling:
require:
Expand Down
6 changes: 0 additions & 6 deletions tests/fixtures/mapping-syntax-error.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,6 @@ users:
scheduling:
require:
- earth
[email protected]:
scheduling:
require:
- pulsar
reject:
- pulsar
.*@vortex.org:
scheduling:
require:
Expand Down
50 changes: 50 additions & 0 deletions tests/fixtures/mapping-user-invalid-tags.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
global:
default_inherits: default

tools:
default:
cores: 2
mem: 8
gpus: 1
env: {}
scheduling:
require: []
prefer:
- general
accept:
reject:
- pulsar
params:
native_spec: "--mem {mem} --cores {cores}"
rules: []

users:
default:
max_cores: 3
max_mem: 4
env: {}
scheduling:
require: []
prefer:
- general
accept:
reject:
- pulsar
rules: []
[email protected]:
scheduling:
require:
- pulsar
reject:
- pulsar

destinations:
local:
runner: local
max_accepted_cores: 4
max_accepted_mem: 16
scheduling:
prefer:
- general
accept:
- pulsar
6 changes: 0 additions & 6 deletions tests/fixtures/mapping-user.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,6 @@ users:
- earth
reject:
- pulsar
[email protected]:
scheduling:
require:
- pulsar
reject:
- pulsar
[email protected]:
max_cores: 4
max_mem: 32
Expand Down
3 changes: 1 addition & 2 deletions tests/fixtures/scenario-too-many-highmem-jobs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,7 @@ users:
rule_helper = RuleHelper(app)
# Find all destinations that support highmem
destinations = [d.dest_name for d in mapper.destinations.values()
if any(d.tpv_dest_tags.filter(tag_value='highmem',
tag_type=[TagType.REQUIRE, TagType.PREFER, TagType.ACCEPT]))]
if 'highmem' in (d.tpv_dest_tags.require + d.tpv_dest_tags.prefer + d.tpv_dest_tags.accept)]
count = rule_helper.job_count(for_user_email=user.email, for_destinations=destinations)
if count > 4:
retval = True
Expand Down
80 changes: 38 additions & 42 deletions tests/test_entity.py
Original file line number Diff line number Diff line change
@@ -1,83 +1,79 @@
import os
import unittest
from tpv.rules import gateway
from tpv.core.entities import Destination
from tpv.core.entities import Tag
from tpv.core.entities import TagType
from tpv.core.entities import Tool
from tpv.core.loader import TPVConfigLoader

from tpv.commands.test import mock_galaxy
from tpv.core.entities import Destination, Tool
from tpv.core.loader import TPVConfigLoader
from tpv.rules import gateway


class TestEntity(unittest.TestCase):

@staticmethod
def _map_to_destination(app, job, tool, user):
tpv_config = os.path.join(os.path.dirname(__file__), 'fixtures/mapping-rule-argument-based.yml')
tpv_config = os.path.join(
os.path.dirname(__file__), "fixtures/mapping-rule-argument-based.yml"
)
gateway.ACTIVE_DESTINATION_MAPPER = None
return gateway.map_tool_to_destination(app, job, tool, user, tpv_config_files=[tpv_config])
return gateway.map_tool_to_destination(
app, job, tool, user, tpv_config_files=[tpv_config]
)

# issue: https://github.com/galaxyproject/total-perspective-vortex/issues/53
def test_all_entities_refer_to_same_loader(self):
app = mock_galaxy.App(job_conf=os.path.join(os.path.dirname(__file__), 'fixtures/job_conf.yml'))
app = mock_galaxy.App(
job_conf=os.path.join(os.path.dirname(__file__), "fixtures/job_conf.yml")
)
job = mock_galaxy.Job()

tool = mock_galaxy.Tool('bwa')
user = mock_galaxy.User('ford', '[email protected]')
tool = mock_galaxy.Tool("bwa")
user = mock_galaxy.User("ford", "[email protected]")

# just map something so the ACTIVE_DESTINATION_MAPPER is populated
self._map_to_destination(app, job, tool, user)

# get the original loader
original_loader = gateway.ACTIVE_DESTINATION_MAPPER.loader

context = {
'app': app,
'job': job
}
context = {"app": app, "job": job}
# make sure we are still referring to the same loader after evaluation
evaluated_entity = gateway.ACTIVE_DESTINATION_MAPPER.match_combine_evaluate_entities(context, tool, user)
assert evaluated_entity.loader == original_loader
evaluated_entity = (
gateway.ACTIVE_DESTINATION_MAPPER.match_combine_evaluate_entities(
context, tool, user
)
)
assert evaluated_entity.evaluator == original_loader
for rule in evaluated_entity.rules:
assert rule.loader == original_loader
assert rule.evaluator == original_loader

def test_destination_to_dict(self):

tpv_config = os.path.join(os.path.dirname(__file__), 'fixtures/mapping-rule-argument-based.yml')
tpv_config = os.path.join(
os.path.dirname(__file__), "fixtures/mapping-rule-argument-based.yml"
)
loader = TPVConfigLoader.from_url_or_path(tpv_config)

# create a destination
destination = loader.destinations["k8s_environment"]
destination = loader.config.destinations["k8s_environment"]
# serialize the destination
serialized_destination = destination.to_dict()
serialized_destination = destination.dict()
# deserialize the same destination
deserialized_destination = Destination.from_dict(loader, serialized_destination)
deserialized_destination = Destination(
evaluator=loader, **serialized_destination
)
# make sure the deserialized destination is the same as the original
self.assertEqual(deserialized_destination, destination)

def test_tool_to_dict(self):
tpv_config = os.path.join(os.path.dirname(__file__), 'fixtures/mapping-rule-argument-based.yml')
tpv_config = os.path.join(
os.path.dirname(__file__), "fixtures/mapping-rule-argument-based.yml"
)
loader = TPVConfigLoader.from_url_or_path(tpv_config)

# create a tool
tool = loader.tools["limbo"]
tool = loader.config.tools["limbo"]
# serialize the tool
serialized_destination = tool.to_dict()
serialized_tool = tool.dict()
# deserialize the same tool
deserialized_destination = Tool.from_dict(loader, serialized_destination)
deserialized_tool = Tool(evaluator=loader, **serialized_tool)
# make sure the deserialized tool is the same as the original
self.assertEqual(deserialized_destination, tool)

def test_tag_equivalence(self):
tag1 = Tag("tag_name", "tag_value", TagType.REQUIRE)
tag2 = Tag("tag_name2", "tag_value", TagType.REQUIRE)
tag3 = Tag("tag_name", "tag_value1", TagType.REQUIRE)
tag4 = Tag("tag_name", "tag_value1", TagType.PREFER)
same_as_tag1 = Tag("tag_name", "tag_value", TagType.REQUIRE)

self.assertEqual(tag1, tag1)
self.assertEqual(tag1, same_as_tag1)
self.assertNotEqual(tag1, tag2)
self.assertNotEqual(tag1, tag3)
self.assertNotEqual(tag1, tag4)
self.assertNotEqual(tag1, "hello")
self.assertEqual(deserialized_tool, tool)
Loading
Loading