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 get_loader().load() from local couldn't get version correctly #59

Merged
merged 8 commits into from
Oct 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Added
- Make ubiconfig.utils.config_validation.validate_config publicly available via
ubiconfig.validate_config

### Fixed
- Fix LocalLoader couldn't get right version if the argument version of load() is None.

## [v2.1.0] - 2019-10-23

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ def get_requirements():

setup(
name="ubi-config",
version="2.1.0",
version="2.2.0",
author="",
author_email="",
packages=find_packages(exclude=["tests", "tests.*"]),
Expand Down
16 changes: 12 additions & 4 deletions tests/ubiconfig/test_ubi.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,10 @@ def test_load_all_with_error_config(


def test_load_from_local():
loader = ubi.get_loader(TEST_DATA_DIR)
path = os.path.join(TEST_DATA_DIR, "configs/ubi7.1")
loader = ubi.get_loader(path)
# loads relative to given path
config = loader.load("configs/ubi7.1/rhel-atomic-host.yaml")
config = loader.load("rhel-atomic-host.yaml")
assert isinstance(config, UbiConfig)
assert config.version == "7.1"

Expand All @@ -195,13 +196,13 @@ def test_load_from_local_decimal_integrity():


def test_load_from_nonyaml(tmpdir):
somefile = tmpdir.join("some-file.txt")
somefile = tmpdir.mkdir("ubi7").join("some-file.txt")
somefile.write("[oops, this is not valid yaml")

loader = ubi.get_loader(str(tmpdir))

with pytest.raises(yaml.YAMLError):
loader.load("some-file.txt")
loader.load("ubi7/some-file.txt")


def test_load_local_failed_validation():
Expand All @@ -220,6 +221,13 @@ def test_load_all_from_local():
assert isinstance(configs[0], UbiConfig)


def test_load_from_directory_not_named_after_ubi():
with patch("os.path.isdir"):
loader = ubi.get_loader("./ubi7.1a")
with pytest.raises(ValueError):
config = loader.load("file")


def test_load_all_from_local_with_error_configs():
loader = ubi.get_loader(TEST_DATA_DIR)
configs = loader.load_all()
Expand Down
19 changes: 13 additions & 6 deletions ubiconfig/_impl/loaders/local.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging
import os
import re

import yaml
from jsonschema.exceptions import ValidationError
Expand Down Expand Up @@ -40,16 +41,22 @@ def load(self, file_name, version=None):

If version is None, we should get it from its path.
"""
if version is None:
# get version form path, such as configs/ubi7.1/config.yaml, get
# ubi7.1
version = os.path.basename(os.path.dirname(file_name))

if not self._isroot:
file_path = os.path.join(self._path, file_name)
else:
file_path = file_name

if version is None:
# get version from path, such as configs/ubi7.1/config.yaml, get
# ubi7.1.
version = os.path.basename(os.path.dirname(os.path.abspath(file_path)))

if not re.search(r"ubi[0-9]\.[0-9]{1,2}$|ubi[0-9]$", version):
raise ValueError(
"Expect directories named in format ubi[0-9].([0-9]{1,2})$' or ubi[0-9]$, but got %s"
% version
)

LOG.info("Loading configuration file locally: %s", file_path)

with open(file_path, "r") as f:
Expand Down Expand Up @@ -94,7 +101,7 @@ def _get_local_files_map(self):
]
if conf_files:
# if there's yaml files, then it must under some version directory
version = os.path.basename(root)
version = os.path.basename(os.path.abspath(root))
ver_files_map.setdefault(version, []).extend(conf_files)
# the result map is as {'version': ['file1', 'file2', ..]}

Expand Down