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.

Sign up for GitHub

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

Jump to bottom

refactor(fetch): rework force-fetch process #986

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

refactor(fetch): rework force-fetch process #986

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
025fcec
refactor(fetch): Rework fetch process
MatteoVoges Apr 5, 2023
b93d949
Merge branch 'kapicorp:master' into restructure-force-fetch
MatteoVoges Apr 5, 2023
75b5210
Merge branch 'master' into restructure-force-fetch
MatteoVoges Aug 28, 2023
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
75 changes: 47 additions & 28 deletions kapitan/targets.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,14 @@


def compile_targets(
inventory_path, search_paths, output_path, parallel, targets, labels, ref_controller, **kwargs
inventory_path,
search_paths,
output_path,
parallel,
targets,
labels,
ref_controller,
**kwargs,
):
"""
Searches and loads target files, and runs compile_target() on a
Expand Down Expand Up @@ -72,13 +79,16 @@ def compile_targets(
if not targets:
updated_targets = changed_targets(inventory_path, output_path)
logger.debug("Changed targets since last compilation: %s", updated_targets)
if len(updated_targets) == 0:
if not updated_targets:
logger.info("No changes since last compilation.")
return

pool = multiprocessing.Pool(parallel)

try:
# -------------------------------------------------
# Rendering inventory
# -------------------------------------------------
rendering_start = time.time()

# check if --fetch or --force-fetch is enabled
Expand All @@ -92,21 +102,13 @@ def compile_targets(
)
force_fetch = True

# -------------------------------------------------
# Fetch inventory and dependencies
# -------------------------------------------------
if fetch:
# skip classes that are not yet available
target_objs = load_target_inventory(inventory_path, updated_targets, ignore_class_notfound=True)
else:
# ignore_class_notfound = False by default
target_objs = load_target_inventory(inventory_path, updated_targets)

# append "compiled" to output_path so we can safely overwrite it
compile_path = os.path.join(output_path, "compiled")

if not target_objs:
raise CompileError("Error: no targets found")

# fetch inventory
if fetch:
# new_source checks for new sources in fetched inventory items
new_sources = list(set(list_sources(target_objs)) - cached.inv_sources)
while new_sources:
Expand All @@ -126,31 +128,43 @@ def compile_targets(
# reset inventory cache and load target objs to check for missing classes
cached.reset_inv()
target_objs = load_target_inventory(inventory_path, updated_targets, ignore_class_notfound=False)
# fetch dependencies
if fetch:

fetch_dependencies(output_path, target_objs, dep_cache_dir, force_fetch, pool)
# fetch targets which have force_fetch: true
elif not kwargs.get("force_fetch", False):

# otherwise only fetch dependencies which have force_fetch: true
else:
# ignore_class_notfound = False by default
target_objs = load_target_inventory(inventory_path, updated_targets)

fetch_objs = []
# iterate through targets
for target in target_objs:
try:
# get value of "force_fetch" property
dependencies = target["dependencies"]
# dependencies is still a list
for entry in dependencies:
force_fetch = entry["force_fetch"]
if force_fetch:
fetch_objs.append(target)
except KeyError:
# targets may have no "dependencies" or "force_fetch" key
# get value of "force_fetch" property
dependencies = target.get("dependencies")
if not dependencies:
continue
# fetch dependencies from targets with force_fetch set to true

for dep_entry in dependencies:
if not dep_entry.get("force_fetch", False):
# if no fetch is needed remove the dependency and don't fetch it
target["dependencies"].remove(dep_entry)

# fetch dependencies with force_fetch set to true
if target.get("dependencies", []):
fetch_objs.append(target)
Comment on lines +148 to +154
Copy link
Contributor Author

Choose a reason for hiding this comment

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

idk why I reversed the logic here: so if it hasn't the flag, then we remove it...
Should I fix that?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it makes sense to have a way to force on the command line "not to force" in case you are running from a place with no external connectivity

So i'd probably leave it in as it is, we can change it later if it doesn't make sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Absolutely, but I think it would work the same if we say, that: if force_fetch: true --> force_fetch, instead of removing it from the list, if it don't have force_fetch: true

Copy link
Contributor Author

@MatteoVoges MatteoVoges Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

a way to force on the command line "not to force"

So you want a --dont-force-fetch flag?

Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps --offline-mode so it can be a generic way to turn off all attempts to download things?

@ramaro what to you think?

I am thinking of the case where you run it in places with no connectivity or you just don't want to change anything. It would suck if in those cases it kept failing.

Alternatively, we could handle it with an additional:

fail_on_error: false


# fetch the specified dependencies with force-fetch
if fetch_objs:
fetch_dependencies(output_path, fetch_objs, dep_cache_dir, True, pool)

if not target_objs:
raise CompileError("Error: no targets found")

logger.info("Rendered inventory (%.2fs)", time.time() - rendering_start)

# -------------------------------------------------
# Compile targets
# -------------------------------------------------
worker = partial(
compile_target,
search_paths=search_paths,
Expand All @@ -165,6 +179,11 @@ def compile_targets(
# so p is only not None when raising an exception
[p.get() for p in pool.imap_unordered(worker, target_objs) if p]

# -------------------------------------------------
# Write output to files
# -------------------------------------------------
# append "compiled" to output_path so we can safely overwrite it
compile_path = os.path.join(output_path, "compiled")
os.makedirs(compile_path, exist_ok=True)

# if '-t' is set on compile or only a few changed, only override selected targets
Expand Down
Loading