Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

exclude tags and reservedConcurrency from version hash #8212

Merged
merged 12 commits into from Sep 22, 2020
Merged

exclude tags and reservedConcurrency from version hash #8212

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
ecc172c
exclude tags and reservedConcurrency from version hash
Sep 10, 2020
c1adb6a
push prettify changes
Sep 10, 2020
8562f4f
refactor test - use runServerless
Sep 11, 2020
19ab38c
replace variable with comment
Sep 11, 2020
6efc191
better comment
Sep 11, 2020
5f0b585
Merge remote-tracking branch 'upstream/master' into ignore-function-w…
Sep 14, 2020
b424b3c
remove pr link
Sep 14, 2020
613e737
prettify
Sep 14, 2020
16be040
revert .only
Sep 14, 2020
b8dee0e
async/await!!!
Sep 15, 2020
39e31eb
async/await!!! - for real
Sep 15, 2020
e29da47
remove .only
thewizarodofoz Sep 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion lib/plugins/aws/package/compile/functions/index.js
View file
Open in desktop
Expand Up @@ -494,7 +494,12 @@ class AwsCompileFunctions {
});
}).then(() => {
// Include function configuration in version id hash (without the Code part)
const properties = _.omit(_.get(functionResource, 'Properties', {}), 'Code');
const functionWideProperties = ['ReservedConcurrentExecutions', 'Tags'];
const properties = _.omit(
_.get(functionResource, 'Properties', {}),
'Code',
...functionWideProperties
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the added value of creating functionWideProperties variable.

Isn't it simpler to just do _.omit(..., ['Code', 'ReservedConcurrentExecutions', 'Tags'] ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought it would make it more clear as to why these fields are omitted

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thewizarodofoz that's a good point , but it'll probably be fine to solve it with simple inline code comment:

_.omit(..., [
  'Code',
   // Properties applied to function globally (not specific to version or alias)
  'ReservedConcurrentExecutions', 'Tags'
])

_.forOwn(properties, value => {
if (_.isObject(value)) {
versionHash.write(JSON.stringify(value));
Expand Down
49 changes: 49 additions & 0 deletions lib/plugins/aws/package/compile/functions/index.test.js
View file
Open in desktop
Expand Up @@ -2240,6 +2240,55 @@ describe('AwsCompileFunctions', () => {
});
});

it('should not create a new version object if only function-wide configuration changed', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All new tests should be written with help of runServerless util (see https://github.com/serverless/serverless/blob/master/test/README.md)

In this case I would rely on basic function fixture as follows

  1. Setup a fixture with fixture.setup, as e.g. it's done here:

    serverless/lib/utils/analytics/generatePayload.test.js

    Line 18 in d9b91e9

    fixtures.setup('httpApi').then(({ servicePath }) =>
  2. Issue first runServerless and read all generated lambda version ids
  3. Modify reservedConcurrency and tags with help of updateConfig (returned by fixture.setup()) as it's done here:

    serverless/test/integration/apiGateway.test.js

    Lines 191 to 204 in d9b91e9

    await updateConfig({
    provider: {
    tags: {
    foo: 'bar',
    baz: 'qux',
    },
    tracing: {
    apiGateway: true,
    },
    logs: {
    restApi: true,
    },
    },
    });
  4. Re-run runServerless, and confirm that version id didn't change

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @medikoo I refactored the test, but it seems to me the second call to runServerless isn't reading the updated config. I debugged the test and I did see updateConfig updating the serverless.yml in the temp dir, but as you can see in this assertion (which fails), runServerless isn't picking up those changes.

awsCompileFunctions.serverless.service.functions = {
func: {
handler: 'func.function.handler',
},
anotherFunc: {
handler: 'anotherFunc.function.handler',
},
};

const expectedOutputs = {
FuncLambdaFunctionQualifiedArn: {
Description: 'Current Lambda function version',
Value: { Ref: 'FuncLambdaVersionRk2uCHjn9BWyD0yH8GzU5kTmGVjCc6ZZx46sUUI1LQ' },
},
AnotherFuncLambdaFunctionQualifiedArn: {
Description: 'Current Lambda function version',
Value: {
Ref: 'AnotherFuncLambdaVersionha2TZXucQgvNN4zjzNnEFIaTGAQxdp7jICMvFkl0',
},
},
};

return expect(awsCompileFunctions.compileFunctions())
.to.be.fulfilled.then(() => {
expect(
awsCompileFunctions.serverless.service.provider.compiledCloudFormationTemplate.Outputs
).to.deep.equal(expectedOutputs);

// Change configuration
awsCompileFunctions.serverless.service.provider.compiledCloudFormationTemplate = {
Resources: {},
Outputs: {},
};

_.set(awsCompileFunctions, 'serverless.service.functions.func.tags', { MYTAG: 'mytag' });

_.set(awsCompileFunctions, 'serverless.service.functions.func.reservedConcurrency', 1);

return expect(awsCompileFunctions.compileFunctions()).to.be.fulfilled;
})
.then(() => {
// Expect same version hash
expect(
awsCompileFunctions.serverless.service.provider.compiledCloudFormationTemplate.Outputs
).to.deep.equal(expectedOutputs);
});
});

it('should include description under version too if function is specified', () => {
awsCompileFunctions.serverless.service.functions = {
func: {
Expand Down