launchpad-reviewers team mailing list archive

[Ines Almeida <mp+445989@xxxxxxxxxxxxxxxxxx>]

Added a few of my own thoughts about the code

Diff comments:

> diff --git a/upload-queue-processor/requires.py b/upload-queue-processor/requires.py
> new file mode 100644
> index 0000000..7e5add0
> --- /dev/null
> +++ b/upload-queue-processor/requires.py
> @@ -0,0 +1,36 @@
> +# Copyright (C) 2023 Canonical Ltd.
> +
> +from charmhelpers.core import hookenv
> +from charms.reactive import Endpoint, clear_flag, set_flag, when
> +
> +
> +class UploadProcessorRequires(Endpoint):
> +    @when("endpoint.{endpoint_name}.joined")
> +    def handle_joined_unit(self):
> +        # We don't want to make ti available before it's configured
> +        # It is configured, when data is added from the provider side,
> +        # triggering the 'changed' endpoint
> +        hookenv.log("requires:upload-queue-processor joined")
> +
> +    @when("endpoint.{endpoint_name}.changed")
> +    def handle_changed_unit(self):
> +        clear_flag(self.expand_name("{endpoint_name}.configuration.available"))
> +
> +        # Set trigger to configure txpkgupload if configuration from upload
> +        # queueprocessor was received
> +        # We use `fsroot`` here as a check because it's one of the main fields
> +        # configured by the queue processor.
> +        relation = self.relations[0]

Is there a way to ensure there is only one relation for the txpkgupload? We are only expecting one, but I'm not sure how sure we are that there can't be more.

> +        if "fsroot" in relation.units.received:
> +            set_flag(

There might be a better way to fetch the configuration data within the txpkgupload. We are currently, calling something on the lines of this:

```
@when(`<interface>.configuration.available`)
    ...

    uploader_relation = endpoint_from_flag(
        "upload-queue-processor.configuration.available"
    )
    if uploader_relation:
        config.update(uploader_relation._all_joined_units[0].received)
```

This does work, but I'm still looking if there is a better way

> +                self.expand_name("{endpoint_name}.configuration.available")
> +            )
> +        else:
> +            hookenv.log("Waiting for config from the queue processor")
> +        clear_flag(self.expand_name("changed"))
> +
> +    @when("endpoint.{endpoint_name}.departed")
> +    def handle_departed_unit(self):
> +        clear_flag(self.expand_name("{endpoint_name}.configuration.available"))
> +        self.all_departed_units.clear()
> +        clear_flag(self.expand_name("departed"))


-- 
https://code.launchpad.net/~ines-almeida/launchpad-layers/+git/launchpad-layers/+merge/445989
Your team Launchpad code reviewers is requested to review the proposed merge of ~ines-almeida/launchpad-layers:add-pkgupload-interface into launchpad-layers:main.