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

feat(galaxy-ansible): Switching from v2 api version to v3 #24947

Closed
wants to merge 1 commit into from
Closed

feat(galaxy-ansible): Switching from v2 api version to v3 #24947

wants to merge 1 commit into from

Conversation

lmilbaum
Copy link
Contributor

@lmilbaum lmilbaum commented Oct 2, 2023 •
edited
Loading

Changes

Context

Documentation (please check one with an [x])

  • I have updated the documentation, or
  • No documentation update is required

How I've tested my work (please select one)

I have verified these changes via:

  • Code inspection only, or
  • Newly added/modified unit tests, or
  • No unit tests but ran on a real repository, or
  • Both unit tests + ran on a real repository

rarkins
rarkins requested changes Oct 2, 2023
View reviewed changes
lib/modules/manager/gomod/artifacts.ts Outdated Show resolved Hide resolved
@lmilbaum lmilbaum force-pushed the ansible-galaxy branch 2 times, most recently from 7f2c448 to 9bf341e Compare October 2, 2023 10:08
rarkins
rarkins requested changes Oct 2, 2023
View reviewed changes
Copy link
Collaborator

@rarkins rarkins left a comment

Choose a reason for hiding this comment

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

Please test against a public/demo repo to verify it all works as expected

HonkingGoose
HonkingGoose reviewed Oct 2, 2023
View reviewed changes
lib/modules/datasource/galaxy-collection/index.ts Outdated
@@ -36,7 +36,7 @@ export class GalaxyCollectionDatasource extends Datasource {
const [namespace, projectName] = packageName.split('.');

// TODO: types (#22198)
const baseUrl = `${registryUrl}api/v2/collections/${namespace}/${projectName}/`;
const baseUrl = `${registryUrl}api/v3/collections/${namespace}/${projectName}/`;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are you sure this is a fix type PR? Switching to a new API may be a breaking change.

I guess at the very least this kind of change should go in a feat: type PR, so it's not mingled with "real patches"?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I agree this should be a feat.

Also is v3 really returning the exact same response on the same sub path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for your feedback. Will change it to feat.
@secustor I've started to check it and the response is probably different. I don't know typescript enough to understand the errors it throws.
In any case, the v2 version is broken.

@viceice viceice changed the title fix(galaxy-ansible): Switching from v2 api version to v3 feat(galaxy-ansible): Switching from v2 api version to v3 Oct 3, 2023
@lmilbaum
Copy link
Contributor Author

lmilbaum commented Oct 3, 2023

The following error is raised. No idea how to start debugging it. Would appreciate some help.

DEBUG: Datasource unknown error (repository=platform-engineering-org/iac)
       "datasource": "galaxy-collection",
       "packageName": "amazon.aws",
       "err": {
         "message": "Cannot read properties of undefined (reading 'map')",
         "stack": "TypeError: Cannot read properties of undefined (reading 'map')\n    at GalaxyCollectionDatasource.getReleases (/workspaces/renovate/lib/modules/datasource/galaxy-collection/index.ts:71:57)\n    at processTicksAndRejections (node:internal/process/task_queues:95:5)\n    at async /workspaces/renovate/lib/util/cache/package/decorator.ts:119:18\n    at async getRegistryReleases (/workspaces/renovate/lib/modules/datasource/index.ts:77:15)\n    at async fetchReleases (/workspaces/renovate/lib/modules/datasource/index.ts:289:15)\n    at async lookupUpdates (/workspaces/renovate/lib/workers/repository/process/lookup/index.ts:115:56)\n    at async withLookupStats (/workspaces/renovate/lib/workers/repository/process/fetch.ts:27:18)\n    at async fetchDepUpdates (/workspaces/renovate/lib/workers/repository/process/fetch.ts:72:30)\n    at async /workspaces/renovate/node_modules/.pnpm/[email protected]/node_modules/p-map/index.js:57:22"
       }

@rarkins
Copy link
Collaborator

rarkins commented Oct 3, 2023

I think it's far more complicated than search/replace v2 to v3, we need to plan it in an issue first

@rarkins rarkins closed this Oct 3, 2023
@rarkins
Copy link
Collaborator

rarkins commented Oct 3, 2023

Created feature request: #24981

@lmilbaum lmilbaum deleted the ansible-galaxy branch October 3, 2023 08:35
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@HonkingGoose HonkingGoose HonkingGoose left review comments

@secustor secustor secustor left review comments

@rarkins rarkins rarkins requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lmilbaum @rarkins @secustor @HonkingGoose