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

feat(manager): Support poetry custom repositories. #4524

Merged
merged 11 commits into from
Sep 30, 2019

Conversation

gilbsgilbs
Copy link
Contributor

@@ -88,12 +88,12 @@ function extractFromSection(

function extractRegistries(pyprojectfile: PoetryFile): string[] {
const sources = pyprojectfile.tool.poetry.source;
const registryUrls: string[] = ['https://pypi.org/pypi/'];
Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually we prefer to handle defaults within datasource, not manager. Otherwise it's too hard to override the default registry URL in Renovate config.

Copy link
Contributor Author

@gilbsgilbs gilbsgilbs Sep 21, 2019

Choose a reason for hiding this comment

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

Do you think 0256ad0 is enough? Unless I'm missing something, I don't think I can easily use the default registry from the pypi datasource if I have other registries to add. Hence, I've done something similar to what pip_requirements manager does.

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.

Review suggested updates

const registryUrls: string[] = ['https://pypi.org/pypi/'];
for (const source of sources) {
if (source.url) {
registryUrls.push(source.url);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we de-dupe this in case the user configures pypi.org explicitly?

Copy link
Contributor Author

@gilbsgilbs gilbsgilbs Sep 29, 2019

Choose a reason for hiding this comment

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

Sorry, I misread this comment as a statement instead of a question. I committed code that dedupes, but I don't think it is necessary as pypi datasource will only append pypi.org if registryUrls is empty. If poetry manager adds pypi.org itself, pypi datasource won't add any other datasource. Let me revert this.

Edit: unless you meant “configures pypi.org explicitly” in pyproject.toml, which I think is a bit silly 🤔 . Anyways, it is not a bad thing to dedup I think. Let's not revert after all.

@rarkins rarkins merged commit 0fe2f1b into renovatebot:master Sep 30, 2019
@rarkins
Copy link
Collaborator

rarkins commented Sep 30, 2019

Thanks @gilbsgilbs !

@renovate-bot
Copy link
Collaborator

🎉 This PR is included in version 19.54.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support Poetry custom repositories
3 participants