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.

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

Pipelines: Backend re-work #2125

Merged

Conversation

Pezmc
Copy link
Contributor

@Pezmc Pezmc commented May 11, 2023

Merge target is feature branch 2075-application-pipelines (#2094)

Description

All behind the scenes changes, the front-end presents exactly the same as in #2094, bugs and all.

  • Re-works the backend to use a through table to connect instances to pipeline stages
  • Explicitly creates all the required tables as part of a migration
  • Updates the backend to allow multiple instances per pipeline stage
  • Updates the frontend to only allow one instance per pipeline stage (for now)
  • Adds some validations for instances and guards against instance being null at pipeline stage creation time
  • Small misc cleanup
Screenshot 2023-05-11 at 15 31 35

Related Issue(s)

Part of #2124

Checklist

  • I have read the contribution guidelines
  • [-] Suitable unit/system level tests have been added and they pass
    • Full unit coverage coming as a follow up
  • [-] Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • [-] Changes flowforge.yml?
    • Issue/PR raised on flowforge/helm to update ConfigMap Template
    • Issue/PR raised on flowforge/CloudProject to update values for Staging/Production

Labels

  • Backport needed? -> add the backport label
  • Includes a DB migration? -> add the area:migration label

@Pezmc Pezmc requested review from knolleary and joepavitt May 11, 2023 06:36
@joepavitt
Copy link
Contributor

"the front-end presents exactly the same as in #2094, bugs and all."

Just checking, what bugs are these? Any others than:

  1. No edit functionality
  2. Can still select an already assigned Instance to a Stage?

@joepavitt
Copy link
Contributor

joepavitt commented May 11, 2023

Adding a stage to a new pipeline results in:

Screenshot 2023-05-11 at 10 04 14
[2023-05-11T09:03:37.766Z] INFO: AUDIT: event: application.pipeline.deleted, type: team, id: 1
[2023-05-11T09:04:03.339Z] INFO: AUDIT: event: application.pipeline.created, type: team, id: 1
ValidationError [SequelizeValidationError]: Validation error: this.getInstance is not a function
    at InstanceValidator._validate (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/sequelize/lib/instance-validator.js:50:13)
    at async InstanceValidator._validateAndRunHooks (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/sequelize/lib/instance-validator.js:60:7)
    at async InstanceValidator.validate (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/sequelize/lib/instance-validator.js:54:12)
    at async model.save (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/sequelize/lib/model.js:2426:7)
    at async PipelineStage.create (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/sequelize/lib/model.js:1362:12)
    at async Object.addPipelineStage (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/packages/flowforge/forge/ee/db/controllers/Pipeline.js:16:23)
    at async Object.<anonymous> (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/packages/flowforge/forge/ee/routes/pipeline/index.js:63:21) {
  errors: [
    ValidationErrorItem {
      message: 'this.getInstance is not a function',
      type: 'Validation error',
      path: 'instancesHaveSameApplication',
      value: null,
      origin: 'FUNCTION',
      instance: [PipelineStage],
      validatorKey: 'instancesHaveSameApplication',
      validatorName: null,
      validatorArgs: [],
      original: TypeError: this.getInstance is not a function
          at model.instancesHaveSameApplication (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/packages/flowforge/forge/ee/db/models/PipelineStage.js:21:47)
          at InstanceValidator._invokeCustomValidator (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/sequelize/lib/instance-validator.js:156:31)
          at /Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/sequelize/lib/instance-validator.js:93:28
          at /Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/lodash/lodash.js:4967:15
          at baseForOwn (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/lodash/lodash.js:3032:24)
          at /Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/lodash/lodash.js:4936:18
          at Function.forEach (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/lodash/lodash.js:9410:14)
          at InstanceValidator._customValidators (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/sequelize/lib/instance-validator.js:89:7)
          at InstanceValidator._validate (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/sequelize/lib/instance-validator.js:47:12)
          at InstanceValidator._validateAndRunHooks (/Users/joepavitt/Documents/flowforge/development/flowforge-dev-env/node_modules/sequelize/lib/instance-validator.js:60:18)
    }
  ]
}

@joepavitt
Copy link
Contributor

problem is in ee/db/models/PipelineStage.js - line 21:

const instancesPromise = this.getInstance()

@joepavitt
Copy link
Contributor

Trying to fix this myself, but can't see where this.getInstance() or this.getPipeline() are/were defined?

@Pezmc
Copy link
Contributor Author

Pezmc commented May 11, 2023

Trying to fix this myself, but can't see where this.getInstance() or this.getPipeline() are/were defined?

built in sequelize method

@Pezmc
Copy link
Contributor Author

Pezmc commented May 11, 2023

Just checking, what bugs are these? Any others than:

Those, plus:

  • Stuck loading state if a request fails on create/delete
  • Creating a pipeline stage without an instance causing the pipeline not to render.

Am tracking in #2124

Copy link
Member

@knolleary knolleary left a comment

Choose a reason for hiding this comment

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

Approving to allow progress - but noting the TODOs.

@knolleary knolleary merged commit 5ef2dbc into 2075-application-pipelines May 16, 2023
4 checks passed
@knolleary knolleary deleted the 2075-application-pipelines-db-refactor branch May 16, 2023 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants