terascope / teraslice

Scalable data processing pipelines in JavaScript
https://terascope.github.io/teraslice/
Apache License 2.0
50 stars 13 forks source link

FIx docker build on Teraslice v0.88.0 #3490

Closed godber closed 11 months ago

godber commented 11 months ago

The docker builds are also broken on the new Typescript Teraslice:

https://github.com/terascope/teraslice/actions/runs/7135923416/job/19433543438

godber commented 11 months ago

It's probably this change here that broke the build:

https://github.com/terascope/teraslice/pull/3478/files#diff-5fe2de2b60d8d1b167d645f3f7a9099d403b4279e532bf07a1b2267698f81099L16

Looking at the error log:

ts-scripts publish <action>

Publish npm or docker releases

Positionals:
  action  The publish action to take  [required] [choices: "docker", "npm"]

Options:
      --dry-run                    For testing purposes, don't pushing or publishing  [boolean] [default: false]
      --publish-outdated-packages  Publish packages that may have newer versions  [boolean] [default: false]
  -t, --type                       Depending on the publish action this can be used to define what type of action to take  [string] [choices: "latest", "prerelease", "daily", "dev", "tag"] [default: "latest"]
  -n, --node-version               Node version, there must be a Docker base image with this version (e.g. 18.18.2)  [string] [default: "18.18.2"]
  -h, --help                       Show help  [boolean]
  -v, --version                    Show version number  [boolean]

Examples:
  ts-scripts publish  -t tag docker
  ts-scripts publish  -t dev docker
  ts-scripts publish  -t latest docker
  ts-scripts publish  -n 18.18.2 -t latest docker
  ts-scripts publish  --dry-run docker
  ts-scripts publish  -n 18.18.2 --dry-run docker
  ts-scripts publish  -t tag npm
  ts-scripts publish  -t latest npm
  ts-scripts publish  --dry-run npm
  ts-scripts publish  --skip-reset npm

Error: At least one package must be specified with `terascope.main`
    at publishToDocker (/home/runner/work/teraslice/teraslice/packages/scripts/dist/src/helpers/publish/index.js:84:23)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Error: Process completed with exit code 1.

We should dig into ts-scripts build ... docker.

godber commented 11 months ago

Oh, my bad, it's this change from true to false:

https://github.com/terascope/teraslice/pull/3478/files#diff-5fe2de2b60d8d1b167d645f3f7a9099d403b4279e532bf07a1b2267698f81099R88

godber commented 11 months ago

This is what cause @jsnoble to change that:

https://github.com/terascope/teraslice/blob/master/packages/scripts/src/helpers/sync/configs.ts#L13

sotojn commented 11 months ago

After reviewing the code, I still think the best solution would be to do two things:

My only concern is that I still don't really understand the intended purpose of checking terascope.main in the package.json. This method also filters out packages that don't have a ts.config file in the package.json directory, which would have also filtered out the teraslice package anyways before it was in typescript.

I have a PR implementing this fix here #3492

godber commented 11 months ago

This is the PR that introduced the line you're removing:

https://github.com/terascope/teraslice/commit/4c79bcc9f86379bbf879e068dbe714c30817b81d

The main point seems to be to add the entire function generateTSConfig (which contains the line you remove).

It would make sense that at the time the main package was excluded since it wasn't typescript yet.

As long as when sync is run it doesn't make undesirable changes to ts-config ... does it get run automatically or manually. I vaguely recall it running automatically a lot. Did we do something to bypass running sync elsewhere?

sotojn commented 11 months ago

I ran yarn ts-scripts sync and it did not change any of the ts-config files. I believe it runs automatically when we use bump.

godber commented 11 months ago

This is resolved, thanks @sotojn .

I screwed up the release though, I failed to bump the teraslice version when I made the release.

I've recorded it here:

https://github.com/terascope/teraslice/releases/tag/v0.88.1