launchpad-reviewers team mailing list archive

[Ines Almeida <mp+445989@xxxxxxxxxxxxxxxxxx>]

Added some of my own thoughts and questions 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..f8e5063
> --- /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 it available before it's configured
> +        # It is configured, when data is added from the provider side,
> +        # triggering the 'changed' endpoint
> +        hookenv.log("Txpkgupload interface available. Waiting for config.")
> +
> +    @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. I'm looking into that

> +        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.