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

Include both pgx 0.2.x and pgx 0.4.x in CI image #417

Merged
merged 1 commit into from
May 11, 2022
Merged

Conversation

rtwalker
Copy link
Contributor

We're updating our pgx dependency to 0.4 series, and using cargo-pgx install with pgx 0.4.x inside the update-tester causes problems.

Starting in 0.4, pgx changed it's SQL generation and gives the following error when running cargo pgx install inside the update tester with previous versions of the toolkit checked out:

found pgx 0.2-0.3 series SQL generation while using cargo-pgx 0.4 series.

As a solution, we're changing the docker image used in our CI setup to include 0.2 series version of pgx installed at /pgx/0.2 as well as a 0.4 series version of pgx installed at /pgx/0.4 so that we can run the update tester with the appropriate cargo-pgx subcommand.

We're updating our pgx dependency to 0.4 series, and using `cargo-pgx
install` with pgx 0.4.x inside the update-tester causes problems.

Starting in 0.4, pgx changed it's SQL generation and gives the
following error when running `cargo pgx install` inside the update
tester with previous versions of the toolkit checked out:

  found `pgx` 0.2-0.3 series SQL generation while using `cargo-pgx`
  0.4 series.

As a solution, we're changing the docker image used in our CI setup to
include 0.2 series version of pgx installed at /pgx/0.2 as well as a
0.4 series version of pgx installed at /pgx/0.4 so that we can run the
update tester with the appropriate `cargo-pgx` subcommand.
Copy link
Contributor

@JLockerman JLockerman left a comment

Choose a reason for hiding this comment

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

Looks right to me 👍
(and the CI builder test seems to be running 😋)

@rtwalker
Copy link
Contributor Author

bors r+

@bors
Copy link
Contributor

bors bot commented May 11, 2022

Build succeeded:

@bors bors bot merged commit fc8eff4 into main May 11, 2022
@bors bors bot deleted the rw/new-ci-image branch May 11, 2022 19:56
bors bot added a commit that referenced this pull request May 12, 2022
418: `update-tester` requires two versions of `cargo-pgx` r=rtwalker a=rtwalker

Now that #417 is merged, we can change the `update-tester` crate to require two versions of `pgx` to be installed: 
 * a 0.2-0.3 series pgx aka "old"
 * a 0.4 series or newer pgx (we don't actually use this one yet, but we will after #408)

Rather than insisting on having the `pgx`s installed in a specific location, the update tester takes in paths to the `*/bin/cargo-pgx` subcommands as arguments and then inspects the `Cargo.toml` during the checkout of each release to determine which one to invoke. 

I've also updated the `tools/build` script and the `.github/workflows/ci.yml` to use the new update tester.

Went ahead and bumped the `update-tester` version to `0.2.0` so I could more easily tell the difference while testing locally, though it probably makes little practical difference.

Co-authored-by: Ryan Walker <rwalker@timescale.com>
bors bot added a commit that referenced this pull request May 12, 2022
418: `update-tester` requires two versions of `cargo-pgx` r=rtwalker a=rtwalker

Now that #417 is merged, we can change the `update-tester` crate to require two versions of `pgx` to be installed: 
 * a 0.2-0.3 series pgx aka "old"
 * a 0.4 series or newer pgx (we don't actually use this one yet, but we will after #408)

Rather than insisting on having the `pgx`s installed in a specific location, the update tester takes in paths to the `*/bin/cargo-pgx` subcommands as arguments and then inspects the `Cargo.toml` during the checkout of each release to determine which one to invoke. 

I've also updated the `tools/build` script and the `.github/workflows/ci.yml` to use the new update tester.

Went ahead and bumped the `update-tester` version to `0.2.0` so I could more easily tell the difference while testing locally, though it probably makes little practical difference.

Co-authored-by: Ryan Walker <rwalker@timescale.com>
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.

2 participants