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.

Sign up for GitHub

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

Jump to bottom

Switch to using the new record_ecosystem_versions endpoint. #7517

Merged
merged 1 commit into from
Jul 10, 2023
Merged

Switch to using the new record_ecosystem_versions endpoint. #7517

merged 1 commit into from
Jul 10, 2023

Conversation

jeffwidman
Copy link
Member

@jeffwidman jeffwidman commented Jul 4, 2023 •
edited
Loading

This flips the updater to calling the new record_ecosystem_versions
endpoint.

Additionally, this flips the per-ecosystem implementations of
package_manager_version to this new ecosystem_versions hash.

Note that we expect the shape of the data to be slightly different in
the long term for these metrics... ie, we plan to record a
min/max/raw or something along those lines for the range of
versions supported by the manifest.

However, changing that will be done in separate PR's for easier
review/discussion/audit of those individual ecosystems.

Related:

@github-actions github-actions bot added L: ruby:bundler RubyGems via bundler L: go:modules Golang modules L: rust:cargo Rust crates via cargo L: javascript labels Jul 4, 2023
This was referenced Jul 4, 2023
Merged
Merged
@jeffwidman jeffwidman changed the title Switch to using record ecosystem versions endpoint Switch to using the new record_ecosystem_versions endpoint. Jul 4, 2023
@jeffwidman jeffwidman force-pushed the switch-to-using-record-ecosystem-versions-endpoint branch from 96be1bb to bed59e8 Compare July 4, 2023 05:14
@github-actions github-actions bot removed L: ruby:bundler RubyGems via bundler L: go:modules Golang modules L: rust:cargo Rust crates via cargo L: javascript labels Jul 4, 2023
@jeffwidman jeffwidman marked this pull request as ready for review July 4, 2023 05:16
@jeffwidman jeffwidman requested a review from a team as a code owner July 4, 2023 05:16
@jeffwidman jeffwidman force-pushed the switch-to-using-record-ecosystem-versions-endpoint branch 2 times, most recently from c2c28d2 to 1716d2c Compare July 4, 2023 05:47
brrygrdn
brrygrdn approved these changes Jul 4, 2023
View reviewed changes
Copy link
Contributor

@brrygrdn brrygrdn left a comment

Choose a reason for hiding this comment

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

This makes sense to me, 👍🏻

updater/lib/dependabot/api_client.rb Show resolved Hide resolved
bin/dry-run.rb Outdated Show resolved Hide resolved
updater/lib/dependabot/file_fetcher_command.rb Outdated Show resolved Hide resolved
@jeffwidman jeffwidman force-pushed the switch-to-using-record-ecosystem-versions-endpoint branch from ad0b2a3 to 1c173d8 Compare July 4, 2023 17:41
jeffwidman
jeffwidman commented Jul 4, 2023
View reviewed changes
common/lib/dependabot/file_fetchers/base.rb
@@ -102,7 +102,7 @@ def clone_repo_contents
raise Dependabot::RepoNotFound, source
end

def package_manager_version
def ecosystem_versions
Copy link
Member Author

Choose a reason for hiding this comment

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

This needs a code comment explaining the expected shape of the data, or possibly a doc somewhere for devs, but we're still noodling internally on what we want this to look like, so punting on those docs until later.

Right now just want to get it swapped over to this new endpoint so can start prototyping actual code to see how my design ideas hold up to reality--ie, do we want to record more/different info then I expect.

@jeffwidman jeffwidman force-pushed the switch-to-using-record-ecosystem-versions-endpoint branch from 1c173d8 to ee51f3c Compare July 4, 2023 17:47
@jeffwidman
Copy link
Member Author

These test failures indicate may need to port the callers of this as part of this PR as well... or bury the package manager calls in private methods since the ecosystem_versions() will likely call both package_manager_versions and programming_language_versions :

rspec './spec/dependabot/bundler/file_fetcher_spec.rb[1:1:1:5]' # Dependabot::Bundler::FileFetcher behaves like a dependency file fetcher the class doesn't define any additional public instance methods

@jeffwidman jeffwidman force-pushed the switch-to-using-record-ecosystem-versions-endpoint branch from ee51f3c to 824ea46 Compare July 10, 2023 21:06
@github-actions github-actions bot added L: ruby:bundler RubyGems via bundler L: go:modules Golang modules L: rust:cargo Rust crates via cargo L: javascript labels Jul 10, 2023
@jeffwidman jeffwidman force-pushed the switch-to-using-record-ecosystem-versions-endpoint branch from 824ea46 to e508a7e Compare July 10, 2023 21:20
@jeffwidman @brrygrdn
Switch to the record_ecosystem_versions endpoint
0c2aeae 
This flips the `updater` to calling the new `record_ecosystem_versions`
endpoint.

Additionally, this flips the per-ecosystem implementations of
`package_manager_version` to this new `ecosystem_versions` hash.

Note that we expect the shape of the data to be slightly different in
the long term for these metrics... ie, we plan to record a
`min`/`max`/`raw` or something along those lines for the range of
versions supported by the manifest.

However, changing that will be done in separate PR's for easier
review/discussion/audit of those individual ecosystems.

Co-authored-by: Barry Gordon <[email protected]>
@jeffwidman jeffwidman force-pushed the switch-to-using-record-ecosystem-versions-endpoint branch from e508a7e to 0c2aeae Compare July 10, 2023 21:38
@jeffwidman jeffwidman enabled auto-merge (squash) July 10, 2023 21:42
@jeffwidman jeffwidman merged commit 781783f into main Jul 10, 2023
@jeffwidman jeffwidman deleted the switch-to-using-record-ecosystem-versions-endpoint branch July 10, 2023 21:50
jeffwidman added a commit to dependabot/cli that referenced this pull request Jul 11, 2023
@jeffwidman
Migrate RecordPackageManagerVersion to RecordEcosystemVersions
3f6ceef 
This endpoint was renamed in dependabot/dependabot-core#7517
and the accompanying internal API change.

So the mock here needs updating. Also updated the struct.
@jeffwidman jeffwidman mentioned this pull request Jul 11, 2023
Merged
jeffwidman added a commit to dependabot/cli that referenced this pull request Jul 11, 2023
@jeffwidman
Migrate RecordPackageManagerVersion to RecordEcosystemVersions
cad6c54 
This endpoint was renamed in dependabot/dependabot-core#7517
and the accompanying internal API change.

So the mock here needs updating. Also updated the struct.
jeffwidman added a commit to dependabot/cli that referenced this pull request Jul 11, 2023
@jeffwidman
Migrate RecordPackageManagerVersion to RecordEcosystemVersions (#145
8e90366 
)

This endpoint was renamed in dependabot/dependabot-core#7517
and the accompanying internal API change.

So the mock here needs updating. Also updated the struct.
jeffwidman added a commit to dependabot/smoke-tests that referenced this pull request Jul 11, 2023
@jeffwidman
Migrate to the new record_ecosystem_versions endpoint
07475bd 
As of dependabot/dependabot-core#7517 we're now
using a `record_ecosystem_versions` endpoint instead of
`record_package_manager_versions`.

So this updates the test expectations accordingly.
@jeffwidman jeffwidman mentioned this pull request Jul 11, 2023
Merged
jeffwidman added a commit to dependabot/cli that referenced this pull request Jul 11, 2023
@jeffwidman
ecosystem_versions replaces package_manager field
eb8c961 
I confused myself in the previous two PR's... the `Ecosystem` field is
actually removed, and the `PackageManager` field is replaced by the
`EcosystemVersions` field.

For background context:
* #145
* #146
* dependabot/dependabot-core#7492
* dependabot/dependabot-core#7517

Caught this discrepancy over here:
* https://github.com/dependabot/smoke-tests/pull/98/files#r1260099204
jeffwidman added a commit to dependabot/cli that referenced this pull request Jul 11, 2023
@jeffwidman
ecosystem_versions replaces package_manager field
10f1552 
I confused myself in the previous two PR's... the `Ecosystem` field is
actually removed, and the top-level `PackageManager` field is replaced by the
`EcosystemVersions` field.

As explained in #146, there may
be an inner `PackageManager` field, which is optional. But no need to
map it out in the struct, at least not yet.

For background context:
* #145
* #146
* dependabot/dependabot-core#7492
* dependabot/dependabot-core#7517

Caught this discrepancy over here:
* https://github.com/dependabot/smoke-tests/pull/98/files#r1260099204
@jeffwidman jeffwidman mentioned this pull request Jul 11, 2023
Merged
jeffwidman added a commit to dependabot/cli that referenced this pull request Jul 11, 2023
@jeffwidman
ecosystem_versions replaces package_manager field
8a8dbc0 
I confused myself in the previous two PR's... the `Ecosystem` field is
actually removed, and the top-level `PackageManager` field is replaced by the
`EcosystemVersions` field.

As explained in #146, there may
be an inner `PackageManager` field, which is optional. But no need to
map it out in the struct, at least not yet.

For background context:
* #145
* #146
* dependabot/dependabot-core#7492
* dependabot/dependabot-core#7517

Caught this discrepancy over here:
* https://github.com/dependabot/smoke-tests/pull/98/files#r1260099204
jeffwidman added a commit to dependabot/cli that referenced this pull request Jul 11, 2023
@jeffwidman
ecosystem_versions replaces package_manager field (#147)
b6f07ab 
I confused myself in the previous two PR's... the `Ecosystem` field is
actually removed, and the top-level `PackageManager` field is replaced by the
`EcosystemVersions` field.

As explained in #146, there may
be an inner `PackageManager` field, which is optional. But no need to
map it out in the struct, at least not yet.

For background context:
* #145
* #146
* dependabot/dependabot-core#7492
* dependabot/dependabot-core#7517

Caught this discrepancy over here:
* https://github.com/dependabot/smoke-tests/pull/98/files#r1260099204
jeffwidman added a commit to dependabot/smoke-tests that referenced this pull request Jul 11, 2023
@jeffwidman
Migrate to the new record_ecosystem_versions endpoint
688772a 
As of dependabot/dependabot-core#7517 we're now
using a `record_ecosystem_versions` endpoint instead of
`record_package_manager_versions`.

So this updates the test expectations accordingly.
jeffwidman added a commit to dependabot/smoke-tests that referenced this pull request Jul 11, 2023
@jeffwidman
Migrate to the new record_ecosystem_versions endpoint (#98)
e630a54 
As of dependabot/dependabot-core#7517 we're now
using a `record_ecosystem_versions` endpoint instead of
`record_package_manager_versions`.

So this updates the test expectations accordingly.
brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023
@jeffwidman @brrygrdn
Switch to the record_ecosystem_versions endpoint (dependabot#7517)
5654ce5 
This flips the `updater` to calling the new `record_ecosystem_versions`
endpoint.

Additionally, this flips the per-ecosystem implementations of
`package_manager_version` to this new `ecosystem_versions` hash.

Note that we expect the shape of the data to be slightly different in
the long term for these metrics... ie, we plan to record a
`min`/`max`/`raw` or something along those lines for the range of
versions supported by the manifest.

However, changing that will be done in separate PR's for easier
review/discussion/audit of those individual ecosystems.

Co-authored-by: Barry Gordon <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brrygrdn brrygrdn brrygrdn approved these changes

Assignees
No one assigned
Labels
L: go:modules Golang modules L: javascript L: ruby:bundler RubyGems via bundler L: rust:cargo Rust crates via cargo
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jeffwidman @brrygrdn