python language support #122
Conversation
hdonnay
force-pushed
the
hank/pip
branch
2 times, most recently
from
0e4e937
to
5a78d3b
Compare
There was a problem hiding this comment.
I think some docs are in order for this PR as some new features have been added. The docs should reflect how a developer can go about using the new custom versioning type in their own code, what it means to be "authoritative" and any changes to the current matching flow should be expressed.
| @@ -147,9 +147,13 @@ func (f *fetcher) fetch(ctx context.Context, layer *claircore.Layer) error { | |||
| br := bufio.NewReader(tr) | |||
| // Look at the content-type and optionally fix it up. | |||
| ct := resp.Header.Get("content-type") | |||
| log.Debug(). | |||
| Str("content-type", ct). | |||
| Msg("reported content-type") | |||
| switch { | |||
| case ct == "" || | |||
There was a problem hiding this comment.
nit: single case switch statement is equivalent to "if" with more code.
| var buf strings.Builder | ||
| b := make([]byte, 0, 16) // 16 byte wide scratch buffer | ||
|
|
||
| buf.WriteByte('{') |
There was a problem hiding this comment.
I do not love that this code exists here, as it feels like something the Range type should provide. However I understand the placement here since this is the "postgres" representation of the data.
What if we use a defined type:
type PGRange claircore.Range
func (r *PGRange) String() {
...
}
This cleans up the code, keeps the string representation in the pg package, and the method set on this defined type only contains the string() method.
| v := &record.Package.NormalizedVersion | ||
| var lit strings.Builder | ||
| b := make([]byte, 0, 16) | ||
| lit.WriteString("'{") |
There was a problem hiding this comment.
A defined type on "Version" can remove this builder code from the body of the query code.
| "github.com/quay/claircore/test" | ||
| ) | ||
|
|
||
| func Test_GetQueryBuilder_Deterministic_Args(t *testing.T) { | ||
| const ( |
| // | ||
| // The Dev revision is promoted earlier in the int slice if there's no Pre or | ||
| // Post revision, and sorts as earlier than a Pre revision. | ||
| func (v *Version) Version() (c claircore.Version) { |
There was a problem hiding this comment.
nit: methods which transform type from one package to another I see normally named "ToVersion" or "ToCCVersion" There's a bit of overloading of this term.
| // | ||
| // The result will be 0 if a==b, -1 if a < b, and +1 if a > b. If the Versions | ||
| // are of different kinds, the Kinds will be compared lexographically. | ||
| func (v *Version) Compare(x *Version) int { |
There was a problem hiding this comment.
If we hardcode a compare method, what is the point of having "kind" on our Version? We are essentially hard coding one "kind" a "kind" that sorts linearly from 0-9
There was a problem hiding this comment.
The point of kind is so that application code can ensure it's making not making nonsensical comparisons, like trying to compare a pep440 version with an rpm version, as they may encode differently.
There was a problem hiding this comment.
I do not understand how something can encode differently, but still use this compare method which infers encoding.
| } | ||
| } | ||
|
|
||
| func NewCoalescer(_ context.Context) (indexer.Coalescer, error) { |
| @@ -19,7 +19,8 @@ func (*Matcher) Name() string { | |||
|
|
|||
| // Filter implements driver.Matcher. | |||
| func (*Matcher) Filter(record *claircore.IndexRecord) bool { | |||
| return record.Distribution.DID == "rhel" | |||
| return record.Distribution != nil && | |||
There was a problem hiding this comment.
why this random change and only for rhel?
There was a problem hiding this comment.
It was the only instance I ran into while testing.
There was a problem hiding this comment.
makes me a bit skeptical that it maybe an issue somewhere else, maybe just confirm its not.
python/matcher.go
Outdated
| _ driver.VersionFilter = (*Matcher)(nil) | ||
| ) | ||
|
|
||
| // Matcher attempts to |
Prior to this change, package entries with lots of files (like the python runtime) would exceed the buffer size of the bufio.Scanner. This commit operates on the backing bytes directly instead of using a Scanner.
This PR adds support for scanning and matching python packages.