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

prefetching-dependencies update #195

Merged
merged 4 commits into from
Dec 17, 2024
Merged

prefetching-dependencies update #195

merged 4 commits into from
Dec 17, 2024

Conversation

slimreaper35
Copy link
Contributor

No description provided.

@slimreaper35 slimreaper35 changed the title prefetching-dependencies prefetching-dependencies update Dec 13, 2024
Copy link

🚀 Preview is available at https://pr-195--konflux-docs.netlify.app

@slimreaper35 slimreaper35 force-pushed the devel branch 2 times, most recently from b848672 to a2ec65b Compare December 17, 2024 08:48
Copy link

🚀 Preview is available at https://pr-195--konflux-docs.netlify.app

Copy link
Member

@arewm arewm left a comment

Choose a reason for hiding this comment

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

Thanks. This looks good overall. A couple of comments that would be nice to resolve but are non-blocking.


.Prerequisites

* You have an up-to-date `yarn.lock` file in your source repository. To make sure that you have the latest `yarn.lock` file, or to create a lockfile, run the `yarn install` command. If `yarn.lock` is not up-to-date, Cachi2 will not fetch the dependencies.
Copy link
Member

Choose a reason for hiding this comment

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

Will Cachi2 not fetch any dependencies if the lockfiles are not up to date? Or is the behavior just that the locked dependencies will be fetched (and therefore your desired latest dependencies might not be fetched properly)?

(the comment is relevant for other package managers too)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TL;DR It will fetch for bundler but not for yarn. I have to fix it...

It depends on the implementation details of each package manager. For example for yarn, we use native mechanisms to pre-fetch the dependencies, the lockfile must be up to date. We use the command: yarn install --frozen-lockfile ... which fails in that case. For other package managers like pip, we just manually parse requirements-txt so it does not make sense to do any validation there.

Comment on lines +315 to +328
To prefetch dependencies for a component build, complete the following steps:

. Go to the `.tekton` directory and find the `.yaml` files related to the `*pull request*` and `*push*` processes.
. Configure the hermetic pipeline by adding the following parameters in both `.yaml` files:

+
[source,yaml]
----
spec:
params:
- ...
- name: prefetch-input
value: '{"type": "yarn", "path": "."}'
----
Copy link
Member

Choose a reason for hiding this comment

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

Is this basically the same across all package managers? Would it be possible to abstract the general form to a common location and then just have sections on the individuality of the different managers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, there is a lot of repetition in the docs. Each package manager has the same paragraph:

Verification
From the Konflux Applications view, go to Activity > Pipeline runs.

Go to the pipeline run with Build in the Type column and confirm that the prefetch-dependencies stage displays a green checkmark. This indicates that the build process successfully fetched all dependencies.

From the Konflux Applications view, go to Activity > Latest commits.

So I would propose a follow-up to clean this up and overall make it more concise. Unless you favor doing it here.
But I can delete the Procedure section since it is duplicated for all package managers as well.

Copy link
Member

Choose a reason for hiding this comment

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

A follow up is fine if you don't want to do it righ tnow. You are being consistent.

- swap the package manager columns with the language column
- use correct programming languages (if applicable)

Signed-off-by: Michal Šoltis <[email protected]>
@arewm arewm merged commit 3fa275f into konflux-ci:main Dec 17, 2024
@slimreaper35 slimreaper35 deleted the devel branch December 18, 2024 07:23
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.

2 participants