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

Fall back to eager registry when needed #765

Merged

Conversation

DilumAluthge
Copy link
Contributor

No description provided.

@DilumAluthge DilumAluthge changed the title [WIP] By default, use the eager flavor of the Julia registry from the Julia Pkg Servers [WIP] By default, use the eager flavor of the Julia registry from the Julia Pkg Servers (but try falling back to conservative if an exception is encountered) Dec 5, 2024
@DilumAluthge DilumAluthge changed the title [WIP] By default, use the eager flavor of the Julia registry from the Julia Pkg Servers (but try falling back to conservative if an exception is encountered) By default, use the eager flavor of the Julia registry from the Julia Pkg Servers (but try falling back to conservative if an exception is encountered) Dec 5, 2024
@DilumAluthge DilumAluthge force-pushed the dpa/eager-julia-registry branch from 9ed3231 to 2538d67 Compare December 5, 2024 01:16
@DilumAluthge
Copy link
Contributor Author

@MilesCranmer If it fails with eager, after we set the environment variable to conservative, we need to make sure that juliacall does another Pkg.Registry.update(), otherwise the conservative value won't have any effect.

Does juliacall automatically run Pkg.Registry.update(), or do we need to do so manually ourselves in this package?

@DilumAluthge
Copy link
Contributor Author

Does juliacall automatically run Pkg.Registry.update()

I'm guessing that the answer to this question is yes, based on this line (line 334):

https://github.com/JuliaPy/pyjuliapkg/blob/3a2c66019f098c1ebf84f933a46e7ca70e82792b/src/juliapkg/deps.py#L334-L334

But I'm not very familiar with pyjuliapkg.

pysr/julia_import.py Outdated Show resolved Hide resolved
@DilumAluthge
Copy link
Contributor Author

I realize that we need to do this in every location where a Pkg operation might be needed.

@DilumAluthge
Copy link
Contributor Author

So I'll have to refactor this functionality out, to make it easier to call in multiple locations.

@coveralls
Copy link

coveralls commented Dec 5, 2024

Pull Request Test Coverage Report for Build 12200236348

Details

  • 31 of 31 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 93.52%

Totals Coverage Status
Change from base Build 12156732879: 0.1%
Covered Lines: 1371
Relevant Lines: 1466

💛 - Coveralls

…ia Pkg Servers (but try falling back to `conservative` if an exception is encountered)
@DilumAluthge DilumAluthge force-pushed the dpa/eager-julia-registry branch from 403531e to a40e8c7 Compare December 5, 2024 02:23
@DilumAluthge DilumAluthge marked this pull request as ready for review December 5, 2024 02:23
@DilumAluthge
Copy link
Contributor Author

@MilesCranmer I think this is ready for review now.

pysr/julia_import.py Outdated Show resolved Hide resolved
@DilumAluthge
Copy link
Contributor Author

@MilesCranmer Can you enable CI on the new commit?

@MilesCranmer MilesCranmer changed the title By default, use the eager flavor of the Julia registry from the Julia Pkg Servers (but try falling back to conservative if an exception is encountered) Fall back to eager registry when needed Dec 6, 2024
Copy link
Owner

@MilesCranmer MilesCranmer left a comment

Choose a reason for hiding this comment

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

I made a few changes. Importantly I changed it so that it doesn't change the registry preference by default. Only if it fails to resolve do I switch things to eager.

What do you think of the edits?

@MilesCranmer
Copy link
Owner

Also, added some unittests

@MilesCranmer MilesCranmer merged commit 2b00ada into MilesCranmer:master Dec 6, 2024
18 checks passed
@MilesCranmer
Copy link
Owner

Thanks @DilumAluthge!

@DilumAluthge DilumAluthge deleted the dpa/eager-julia-registry branch December 7, 2024 22:02
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.

3 participants