launchpad-reviewers team mailing list archive

[Ines Almeida <mp+461232@xxxxxxxxxxxxxxxxxx>]

Thank you for the review! Comments were addressed.
I also updated the commit message to mention the service to be consistent with other commit messages in this repo.

Diff comments:

> diff --git a/lp-fetch-service/README.md b/lp-fetch-service/README.md
> new file mode 100644
> index 0000000..65e09cb
> --- /dev/null
> +++ b/lp-fetch-service/README.md
> @@ -0,0 +1,21 @@
> +# Launchpad fetch service
> +
> +This spec deploys Launchpad's fetch service.
> +
> +You can run it locally using Juju's LXD support and Mojo. First, configure
> +your environment:
> +
> +    export MOJO_ROOT="$HOME/.local/share/mojo"
> +    export MOJO_PROJECT=mojo-lp-fetch-service
> +    export MOJO_WORKSPACE=devel
> +    export MOJO_SERIES=jammy
> +    export MOJO_SPEC=git+https://git.launchpad.net/~launchpad/launchpad-mojo-specs/+git/private

Ok makes sense. I have updated it to `$HOME/spec` and I'm guessing I'll have to create that local directory.

Hopefully this can eventually be made public and we can use the public git address here.

> +    export MOJO_STAGE=lp-fetch-service/devel
> +
> +Then run the spec using Mojo:
> +
> +    mojo project-new -c containerless
> +    mojo workspace-new
> +    mojo run
> +
> +You must have python3-yaml installed.
> diff --git a/lp-fetch-service/bundle.yaml b/lp-fetch-service/bundle.yaml
> new file mode 100644
> index 0000000..2811116
> --- /dev/null
> +++ b/lp-fetch-service/bundle.yaml
> @@ -0,0 +1,22 @@
> +{%- if stage_name == "production" %}
> +{%-   set devel = False %}
> +{%- elif stage_name == "staging" %}
> +{%-   set devel = False %}
> +{%- else %}
> +{%-   set devel = True %}
> +{%- endif -%}
> +
> +series: jammy
> +applications:
> +{%- if devel or stage_name == "staging" %}

Great point, updated!

> +  fetch-service:
> +    {#- While the fetch-service charm and snap are not public, we are deploying
> +        a locally built charm. The lines below should be replaced with the path
> +        to charmhub, channel and revision number once the charm and snap are
> +        public. Currently, this expects the charm and snap to be copied to the
> +        path where the bundle.yaml is rendered, in the mojo project folder (eg.
> +        .../mojo/lp-fetch-service/jammy/devel/charms/lp-fetch-service/). #}
> +    charm: "./fetch-service_ubuntu-22.04-amd64.charm"
> +    resources:
> +      snap: "./fetch-service.snap"
> +{%- endif %}
> diff --git a/lp-fetch-service/verify b/lp-fetch-service/verify
> new file mode 100755
> index 0000000..df9e6ef
> --- /dev/null
> +++ b/lp-fetch-service/verify
> @@ -0,0 +1,8 @@
> +#! /bin/sh
> +set -e
> +
> +TOP="${0%/*}"
> +
> +export EXTRA_SKIP_CHECKS="check_swap${EXTRA_SKIP_CHECKS:+|${EXTRA_SKIP_CHECKS}}"

My only reason why we want to do it is that it is skipped in all other specs.

I understand that that might not be a good enough answer here, but I can't really find the reason why it is like that in every other spec as I can't find any mentions of it within comments.

Some info about this check: https://nagios-plugins.org/doc/man/check_swap.html
Some info about swap space: https://help.ubuntu.com/community/SwapFaq

I'll try to investigate and add a comment in the spec with the information I find, but for now I'll keep the SKIP in the code

> +
> +exec "$TOP/utils/verify"


-- 
https://code.launchpad.net/~ines-almeida/launchpad-mojo-specs/+git/private/+merge/461232
Your team Launchpad code reviewers is subscribed to branch ~launchpad/launchpad-mojo-specs/+git/private:master.