Adding worker sample for AFTER_BUILD_INFO_SAVE to set artifact property#26
Adding worker sample for AFTER_BUILD_INFO_SAVE to set artifact property#26nandagopal-jfrog wants to merge 3 commits intojfrog:mainfrom
Conversation
| @@ -0,0 +1,16 @@ | |||
| { | |||
| "name": "build-property-setter", | |||
There was a problem hiding this comment.
Maybe this was already discussed with PM, but this name looks a bit too much generic to me
| "name": "build-property-setter", | |
| "name": "build-mark-latest-artifacts", |
| "sourceCodePath": "./worker.ts", | ||
| "action": "AFTER_BUILD_INFO_SAVE", | ||
| "enabled": false, | ||
| "debug": true |
There was a problem hiding this comment.
I don't think the code gallery takes it into account anyway
| "test": "jest" | ||
| }, | ||
| "license": "ISC", | ||
| "devDependencies": { |
| "compilerOptions": { | ||
| "module": "commonjs", | ||
| "declaration": true, | ||
| "target": "es2017", |
There was a problem hiding this comment.
Because this is what the CLI generates: https://github.com/jfrog/jfrog-cli-platform-services/blob/main/commands/templates/tsconfig.json_template
We are currently using ES2022 on server side indeed.
There was a problem hiding this comment.
@christophermiJfrog Should we fix the CLI, then?
| dependencies: Dependency[]; | ||
| } | ||
|
|
||
| interface Artifact { |
There was a problem hiding this comment.
Is it a good practice to add all the provided field, instead of just add those that are actually in use by the worker?
There was a problem hiding this comment.
It is generated by the CLI: https://github.com/jfrog/jfrog-cli-platform-services/blob/main/commands/init_cmd.go#L180. This gives autocompletion experience while working on the Worker's code
| context: PlatformContext, | ||
| buildName: string, | ||
| buildNumber: string, | ||
| artifactsForPropertyUpdate: Set<ArtifactPathInfo> |
There was a problem hiding this comment.
Is there a reason for the update to use a Set, while the removal uses a Map?
| // Simulate no previous builds | ||
| (context.clients.platformHttp.get as jest.Mock).mockResolvedValueOnce({ | ||
| status: 200, | ||
| data: {buildsNumbers: [], buildInfo: {modules: []}} |
There was a problem hiding this comment.
Since we in the "after build save" event, I guess we will never get an empty list, it should always have at least one build, right?
| }); | ||
|
|
||
| it('should handle past build with module having no artifacts, but log a warning', async () => { | ||
|
|
| modules: [ | ||
| { | ||
| id: 'module1', | ||
| artifacts: [ |
There was a problem hiding this comment.
Add an artifact to cover the loop over artifacts
| (context.clients.platformHttp.get as jest.Mock).mockResolvedValueOnce({ | ||
| status: 200, | ||
| data: { | ||
| buildsNumbers: [{uri: '/12356'}], |
There was a problem hiding this comment.
Add another build to cover the loop over builds
| ); | ||
|
|
||
| for (const artifactInfo of artifactSet) { | ||
| await setProperty(context, artifactInfo.repo, artifactInfo.path); |
There was a problem hiding this comment.
This can also be parallelised.
| @@ -0,0 +1,229 @@ | |||
| import {PlatformContext, Status} from "jfrog-workers"; | |||
There was a problem hiding this comment.
Please also consider cases where volume of artifacts is big. This might cause the Worker to timeout or reach the memory limit.
christophermiJfrog
left a comment
christophermiJfrog
left a comment
There was a problem hiding this comment.
I left few comments, please consider them before we approve this Worker.
3 participants
Added worker sample for build property setting as part of deprecation of plugin buildPropertySetter