-
Notifications
You must be signed in to change notification settings - Fork 14
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
Kickstart the schemas revamp (WIP) #26
Draft
jaimergp
wants to merge
19
commits into
conda:main
Choose a base branch
from
jaimergp:revamp
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
23b74de
Kickstart the schemas revamp
jaimergp e2e0109
move to archive/
jaimergp f4af07c
add pre-commit
jaimergp c8ea81e
add channeldata
jaimergp 25719b2
interrogate
jaimergp 9473efc
add GHA workflow
jaimergp 6aec2e5
matchspec
jaimergp a68c6a3
add environment.yml
jaimergp 2018671
add condarc
jaimergp aa20f40
make optional explicit
jaimergp a60f75c
add repodata patch
jaimergp a0f3f02
simpler approach
jaimergp 6f3140e
add history
jaimergp ce971ce
add requirements.txt
jaimergp bd57b51
add meta yaml draft
jaimergp 231ced7
mark revoke(d) as deprecated
jaimergp 9761791
simplify environment file schema
jaimergp bbb643e
simplify condarc a bit
jaimergp b143c82
typo
jaimergp File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,20 @@ | ||
from pydantic import BaseModel, Extra | ||
from pydantic.main import ModelMetaclass | ||
|
||
|
||
class ExtrasForbiddenModel(BaseModel): | ||
class Config: | ||
extra = Extra.forbid | ||
|
||
|
||
class AllOptional(ModelMetaclass): | ||
def __new__(mcls, name, bases, namespaces, **kwargs): | ||
cls = super().__new__(mcls, name, bases, namespaces, **kwargs) | ||
for field in cls.__fields__.values(): | ||
field.required = False | ||
return cls | ||
|
||
|
||
def export_to_json(model, path): | ||
with open(path, "w") as f: | ||
f.write(model.json(indent=2)) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,24 @@ | ||
""" | ||
WIP | ||
""" | ||
from pydantic import Field | ||
|
||
from ._base import ExtrasForbiddenModel | ||
from .repodata import AllOptionalRepodataRecord | ||
from .types import CondaPackageFileNameStr, PackageFileNameStr, TarBz2PackageFileNameStr | ||
|
||
|
||
class RepodataPatchInstructions(ExtrasForbiddenModel): | ||
packages: dict[TarBz2PackageFileNameStr, AllOptionalRepodataRecord] | ||
"The .tar.bz2 packages in the repodata that will be patched" | ||
packages_conda: dict[CondaPackageFileNameStr, AllOptionalRepodataRecord] = Field( | ||
..., | ||
alias="packages.conda", | ||
) | ||
"The .conda packages in the repodata that will be patched" | ||
remove: set[PackageFileNameStr] | ||
"The packages (conda or tar.bz2) that should be removed from the index" | ||
revoke: set[str] | ||
"Unclear" | ||
patch_instructions_version: int | ||
"Version of the patch instructions schema" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that it doesnt make sense to patch all fields from a RepodataRecord. Changing the name of a package doesnt make sense and changing the hash might break cache implementations. In Rattler we explicity limit the fields that can be patched for that reason. We also didnt encounter fields not covered by our implementation. Might be worth considering!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed that it should be prevented and it's A Bad Idea (tm).
So while the file format seems to accept everything, there are some guidelines place for "reasonable" use and it looks like this is mostly addressed in code reviews, but sometimes we have exceptions. 🤷
I've been thinking whether we want to get descriptive or prescriptive in this PR, and I have mixed feelings about it. For scalars, I've been trying to be more prescriptive, but with structs... just reflecting what (I think) is in use these days. I am not saying I won't change my mind about it, just writing down what the approach have been so far :D
Thanks for the feedback!
Also, are we sure we have a
packages.conda
field? Maybe the readme is outdated, and the seed dict is too?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand where you're coming from. With rattler we definitely choose the prescriptive approach because I feel like there is way too much "legacy" code and supporting all those exceptions is simply harder. But the same logic does not necessarily hold for this project.
The
packages.conda
field is definitely used by theconda-forge-repodata-patches
package. The readme does indeed seem to be outdated. Here is a small excerpt from the latestlinux-64
patches:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the current state, I thin we should be descriptive in the initial implementation: The first step in this effort is to capture what is used currently so that people and projects can refer to the schema to ensure that their software is implemented correctly.
In further steps, we could get more and more prescriptive.
This brings me to a second thing that we'll need to have: versioning. As with everything, at some point, the schema will change.
To enable implementations to reference "the schema" they're using, we'll need to version the schemas. If we don't, we effectively can't change them without breaking something (be that software or user trust).
Edit: Is the
repodata_version
the version of the schema?