-
Notifications
You must be signed in to change notification settings - Fork 3
V2 create basic structure #671 #682
V2 create basic structure #671 #682
Conversation
Signed-off-by: Janine Olear <[email protected]>
Signed-off-by: Janine Olear <[email protected]>
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #682 +/- ##
==========================================
- Coverage 93.90% 92.11% -1.79%
==========================================
Files 9 9
Lines 410 482 +72
Branches 88 102 +14
==========================================
+ Hits 385 444 +59
- Misses 13 20 +7
- Partials 12 18 +6
☔ View full report in Codecov by Sentry. |
Signed-off-by: Janine Olear <[email protected]>
2ea5c05
to
5e3f76d
Compare
Signed-off-by: Janine Olear <[email protected]>
Signed-off-by: Janine Olear <[email protected]>
Signed-off-by: Janine Olear <[email protected]>
43e3931
to
6088c9f
Compare
filename = entry.filename.split("/") | ||
if len(filename) < 3: | ||
print("warn: could not determine region or provider of image: " + entry.filename) | ||
continue | ||
|
||
entry.content["provider"] = filename[1] | ||
entry.content["region"] = filename[2] | ||
entry.content["provider"] = filename[4] |
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.
This is totally fine but I can imagine that these array access values might be confusing in the long run. Maybe we can use namedtuples in the future? It would at least make accessing the individual fields easier.
Just an example:
from collections import namedtuple
APIPath = namedtuple('APIPath', ['api_version', 'distro', 'provider', 'version','region','image_id'])
split_path = "v2/rhel/google/9.0.0/us-east-1/asdasdasdadaf".split('/')
api_path = APIPath(*split_path)
print(api_path.api_version)
print(api_path.image_id)
That api_path could be a property of the DataEntry object or something.
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 agree, that looks way better to maintain. 😄 Is it ok, when I adapt this in another pr? :)
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 created a new issue for it :) #685
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.
Absolutely, thanks for taking care of this.
closes #671