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

dbt deps prerelease install bugs + add install-prerelease parameter to packages.yml #3624

Merged
merged 2 commits into from
Jul 28, 2021
Merged

dbt deps prerelease install bugs + add install-prerelease parameter to packages.yml #3624

merged 2 commits into from
Jul 28, 2021

Conversation

NiallRees
Copy link
Contributor

@NiallRees NiallRees commented Jul 24, 2021

Resolves #3578

Description

Adds logic to compare in semver.py to handle pre-release versions.

Checklist

  • I have signed the CLA
  • I have run this code in development and it appears to resolve the stated issue
  • This PR includes tests, or tests are not required/relevant for this PR
  • I have updated the CHANGELOG.md and added information about my change to the "dbt next" section.

@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 15:12 Inactive
@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 15:12 Inactive
@cla-bot cla-bot bot added the cla:yes label Jul 24, 2021
@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 15:12 Inactive
@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 15:12 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 15:12 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 15:12 Inactive
['0.4.5a1', '0.4.5a2']),
'0.4.5a2')

self.assertEqual(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Testing the specific case raised by @lbk-fishtown in #3578

@@ -134,3 +136,15 @@ def test__resolve_to_specific_version(self):
create_range(None, '<=0.0.5'),
['0.0.3', '0.1.4', '0.0.5']),
'0.0.5')

self.assertEqual(
resolve_to_specific_version(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Testing the specific case raised in #3609

@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 16:23 Inactive
@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 16:23 Inactive
@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 16:23 Inactive
@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 16:23 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 16:23 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 16:23 Inactive
@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 16:27 Inactive
@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 16:27 Inactive
@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 16:27 Inactive
@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 16:27 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 16:27 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 16:27 Inactive
@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 16:29 Inactive
@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 16:29 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 16:29 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 16:29 Inactive
@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 16:29 Inactive
resolve_to_specific_version(
create_range('>=1.0.0', '<1.2.0'),
['1.0.0', '1.1.0a1', '1.1.0', '1.2.0a1', '1.2.0']),
'1.1.0')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jtcohen6 does the behaviour of the above four test cases make sense to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

Those all look spot on!

resolve_to_specific_version(
create_range('>=1.0.0', '<1.2.0'),
['1.0.0', '1.1.0a1', '1.1.0', '1.2.0a1']),
'1.1.0')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 23:39 Inactive
@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 23:39 Inactive
@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 23:39 Inactive
@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 23:39 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 23:39 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 23:39 Inactive
@NiallRees NiallRees changed the title Fix dbt deps prerelease install bugs dbt deps prerelease install bugs + add install-prerelease parameter to packages.yml Jul 24, 2021
@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 23:41 Inactive
@NiallRees NiallRees temporarily deployed to Bigquery July 24, 2021 23:41 Inactive
@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 23:41 Inactive
@NiallRees NiallRees temporarily deployed to Redshift July 24, 2021 23:41 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 23:41 Inactive
@NiallRees NiallRees temporarily deployed to Snowflake July 24, 2021 23:41 Inactive
Copy link
Contributor

@jtcohen6 jtcohen6 left a comment

Choose a reason for hiding this comment

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

Heroic work @NiallRees !!

Thanks for diving into the semver logic, and big props for doing it on such a quick turnaround. We can get this in for v0.20.1—and I agree that's the right place for this. It isn't a breaking change, except in the sense of changing a behavior that's buggy + unintuitive, and this will grant package maintainers much greater peace of mind going forward.

resolve_to_specific_version(
create_range('>=1.0.0', '<1.2.0'),
['1.0.0', '1.1.0a1', '1.1.0', '1.2.0a1', '1.2.0']),
'1.1.0')
Copy link
Contributor

Choose a reason for hiding this comment

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

Those all look spot on!

@lbk-fishtown
Copy link

Thanks Niall and Jeremy for looking into this one!

@jtcohen6 jtcohen6 merged commit 52ec790 into dbt-labs:develop Jul 28, 2021
jtcohen6 pushed a commit that referenced this pull request Jul 28, 2021
…o packages.yml (#3624)

* Fix dbt deps prerelease install bugs

* Add install-prerelease parameter to hub packages in packages.yml

Contributors:
- [@NiallRees](https://github.com/NiallRees) ([#3623](https://github.com/dbt-labs/dbt/pull/3623))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what happened here 🙈 #3639

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries! This happens all the time...

jtcohen6 pushed a commit that referenced this pull request Jul 28, 2021
Fix a typo introduced in #3624
jtcohen6 pushed a commit that referenced this pull request Jul 28, 2021
Fix a typo introduced in #3624
@NiallRees NiallRees deleted the nw/dbt_deps_prerelease_bugfixes branch July 28, 2021 10:59
iknox-fa pushed a commit that referenced this pull request Feb 8, 2022
…o packages.yml (#3624)

* Fix dbt deps prerelease install bugs

* Add install-prerelease parameter to hub packages in packages.yml

automatic commit by git-black, original commits:
  52ec790
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package resolution is inconsistent for prerelease versions
3 participants