serverless / serverless-python-requirements

⚡️🐍📦 Serverless plugin to bundle Python packages
MIT License
1.11k stars 290 forks source link

Support per-function requierments with shared code #767

Open markonose opened 1 year ago

markonose commented 1 year ago

Is there an existing issue for this?

Use case description

See: https://github.com/serverless/serverless-python-requirements/issues/734

Besides it being a bug report, it describes the use case, also see: https://github.com/serverless/serverless-python-requirements/issues/168 https://github.com/serverless/serverless-python-requirements/issues/435

Currently I find the behavior of per function requirements odd, because to me it seems it;s basically a more performant? implementation of https://www.serverless.com/framework/docs/guides/compose

As it uses the module as the entry point and thus you can not use shared code.

Project structure:

- serverless.yml
- application/**
- domain/**
- infrastructure/**
- web
  - api.py
  - requirements.txt
- events
  - handler.py
  - requirements.txt

serverless.yml:

service: service

provider:
  name: aws
  runtime: python3.9

functions:
  RESTAPI:
    handler: api.handler
    module: web
    package:
      patterns:
        - 'web/**'
  EventConsumer:
    handler: handler
    module: events
    package:
      patterns:
        - 'events/**'

package:
  individually: true
  patterns:
    - '!*'
    - '!*/**'
    - 'application/**'
    - 'domain/**'
    - 'infrastructure/**'

plugins:
  - serverless-python-requirements

So currently packaging this results in the following artifacts:

- events
  - requirements/**
  - requirements.txt
- web
  - requirements/**
  - requirements.txt
- cloudformation-template.json
- events-service-stage-EventConsumer.zip
- EventConsumer.zip
- RESTAPI.zip
- serverless-state.json
- web-service-stage-RESTAPI.zip

The RESTAPI zip file contains:

- application/**
- domain/**
- infrastructure/**
- web/**

so the files and folderz defined in package patterns.

and the web-service-stage-RESTAPI zip file contains the web requirements and the contents of the web folder in our project.

When deploying the zips with the fully qualified name get deployed. Thus you can not share code, because as it is specified in the documentation, the module folder becomes the root of the zip of the function.

Currently there is a workaround that people are using the vendor option if only one folder is shared, but this approach does not work for multiple folders

Proposed solution (optional)

So a general idea would be to combine the zip named as the fully qualified function name and the zip named as the function excluding the folder named as the module. But here there could be an issue if there would be no patterns supplied in the root package field.

So I would propose that the merging/copying would only happen if there are patterns defined in the root package field and also including an extra field in custom pythonRequirements or maybe inside the function definitions, something along the lines of includeNonModuleFiles to ensure no breaking changes.

I'm wondering if this is possible or if there are any issues with this approach? Have not looked at the code yet, so no idea if my approach would be feasible. Also is this a use case that this plugin would even want to support?

markonose commented 1 year ago

So having had a look at the code, extending this behavior should be quite trivial. The existing code already copies files from the function.zip, but it filters based on the module, and moves the module to the root.

/**
 * Remove all modules but the selected module from a package.
 * @param {string} source path to original package
 * @param {string} target path to result package
 * @param {string} module module to keep
 * @return {Promise} the JSZip object written out.
 */
function moveModuleUp(source, target, module) {
  const targetZip = new JSZip();

  return fse
    .readFileAsync(source)
    .then((buffer) => JSZip.loadAsync(buffer))
    .then((sourceZip) =>
      sourceZip.filter(
        (file) =>
          file.startsWith(module + '/') ||
          file.startsWith('serverless_sdk/') ||
          file.match(/^s_.*\.py/) !== null
      )
    )
    .map((srcZipObj) =>
      zipFile(
        targetZip,
        srcZipObj.name.startsWith(module + '/')
          ? srcZipObj.name.replace(module + '/', '')
          : srcZipObj.name,
        srcZipObj.async('nodebuffer')
      )
    )
    .then(() => writeZip(targetZip, target));
}

The moveModuleUp function could take in a new parameter, and if true it would return Object.values(sourceZip.files) instead of filtering.

doulighan commented 1 year ago

+1

alka1000 commented 1 year ago

+1

martinezpl commented 8 months ago

+1