canonical / rockcraft

Tool to create OCI Images using the language from Snapcraft and Charmcraft.
GNU General Public License v3.0
37 stars 44 forks source link

Merge extension fields in django-framework/install-app #566

Closed javierdelapuente closed 5 months ago

javierdelapuente commented 6 months ago

For the django-framework extension, in the part django-framework/install it should be possible to customise some fields, like override-build and at the same time let the extension set the rest of fields.

For that, if the django-framework/install part is in the rockcraft.yaml file, update the fields in the extension that are not set in the file rockcraft.yaml.

javierdelapuente commented 6 months ago

More information is needed for this. The extension is an all or nothing, if we do this, making future changes to the extension highly risks breaking people who have decided to use this

My use case for this change is to be able to change one of the build steps of the step django-framework/install-app without having to copy/paste the rest of fields (you can see my desired rockcraft.yaml file here. This way, the step can be customised without having to rewrite those fields (in a kind of natural way I think).

However as you comment, it slightly changes the "interface". Do you think it would be better to repeat the build steps in the rockcraft.yaml file instead?

javierdelapuente commented 5 months ago

More information is needed for this. The extension is an all or nothing, if we do this, making future changes to the extension highly risks breaking people who have decided to use this

My use case for this change is to be able to change one of the build steps of the step django-framework/install-app without having to copy/paste the rest of fields (you can see my desired rockcraft.yaml file here. This way, the step can be customised without having to rewrite those fields (in a kind of natural way I think).

However as you comment, it slightly changes the "interface". Do you think it would be better to repeat the build steps in the rockcraft.yaml file instead?

Reviewing the published docs (for Flask, not for Django, as Django is currently experimental), in flask-framework/install-app, it says that you can specify the property prime in the part flask-framework/install-app. Looking at the code, you can override any property, let's say for example override-pull. This PR makes the two extensions a bit closer, although not the same. What do you think @sergiusens ?

jdkandersson commented 5 months ago

I see the point. The issue we found is that there are reasonable things the user might want to do. For example:

@javierdelapuente I wonder, could this be done in another part so that the part managed by the extension isn't impacted? The only one I can think of is the prime, but I think we discussed that shouldn't be needed anymore?

javierdelapuente commented 5 months ago

I see the point. The issue we found is that there are reasonable things the user might want to do. For example:

  • expand/ generate static files
  • change permissions of files

@javierdelapuente I wonder, could this be done in another part so that the part managed by the extension isn't impacted? The only one I can think of is the prime, but I think we discussed that shouldn't be needed anymore?

Sure, it can be done in other part. It may require using the stage/prime directories to generate the static files and the permission property will not be used (we will probably have to run the chown command instead).

I will close this PR and do it in another part in NetBox. I still think we should review which extension fields we can override as Django does not allow any and Flask allows all of them. For my use case, it is cleaner overriding the fields, but I understand that this topic may require more discussion. Thanks for the inputs!