Finds last rev that built, and caches github #97

CarlFK · 2018-02-06T07:53:47Z

if the target isn't found,

find_last_rev(args, possible_revs) - prints the last ver and exits.

python3 download-prebuilt-firmware.py --platform opsis
Did not find target hdmi2usb for platform opsis at rev v0.0.4-132-g8daff41 (found base, net)
try try try.,,,
found at rev v0.0.4-122-gbc7feb5
(returns to prompt)

so now you can decide to get it:

python3 download-prebuilt-firmware.py --platform opsis --rev v0.0.4-122-gbc7feb5
...
Downloading to: image-gateware+bios+firmware.v0.0.4-122-gbc7feb5.opsis.hdmi2usb.lm32.bin

ls_github(url) now saves its data to github.pickle -
2. special case for the top level dir. Age it - reload after 20 min.

…bad.)

mithro · 2018-02-06T08:00:13Z

bin/download-prebuilt-firmware.py

 import sys
 import time
 import urllib.request

+class Target_not_found(Exception):


class TargetNotFound(Exception):

mithro · 2018-02-06T08:01:16Z

bin/download-prebuilt-firmware.py

+    archive_url = get_url(args)
+
+    for rev in possible_revs:
+


To many new lines....

mithro · 2018-02-06T17:12:09Z

bin/download-prebuilt-firmware.py

 import doctest
 import json
 import os
+import pickle
+from pprint import pprint
 import sys
 import time
 import urllib.request


Can you order the imports

[H306] Alphabetically order your imports by the full module path. Organize your imports according to the Import order template and Real-world Import Order Examples below. For the purposes of import order, OpenStack projects other than the one to which the file belongs are considered “third party”. Only imports from the same Git repo are considered “project imports”

import a_standard import b_standard import a_third_party import b_third_party from a_soc import f from a_soc import g from b_soc import d

https://pypi.python.org/pypi/isort/

mithro · 2018-02-06T17:12:58Z

@CarlFK - I'll make you into a proper Python developer yet :-P

mithro · 2018-02-07T04:23:30Z

I just disable the warning about pickle.

mithro

Can you fix the title? BTW For WIP pull requests, please put WIP in the title.

Otherwise, with these minor fixes I'm happy to merge.

mithro · 2018-02-07T16:53:39Z

bin/download-prebuilt-firmware.py

@@ -193,11 +227,33 @@ def get_targets(args, rev, targets_url):
        print("Did not find target {} for platform {} at rev {} (found {})".
              format(args.target, args.platform, rev,
                     ", ".join(possible_targets)))
-        sys.exit(1)
+        # pprint(possible_targets)


Remove comment.

CarlFK · 2018-02-07T17:26:23Z

On Wed, Feb 7, 2018 at 10:55 AM, Tim Ansell ***@***.***> wrote: ***@***.**** approved this pull request. Can you fix the title? BTW For WIP pull requests, please put WIP in the title. Otherwise, with these minor fixes I'm happy to merge. ------------------------------ In bin/download-prebuilt-firmware.py <#97 (comment)> : > return possible_targets +def find_last_rev(args, possible_revs): So many new lines :-P

pep8 made me do it. 2 between each func def, right?

------------------------------ In bin/download-prebuilt-firmware.py <#97 (comment)> : > @@ -193,11 +227,33 @@ def get_targets(args, rev, targets_url): print("Did not find target {} for platform {} at rev {} (found {})". format(args.target, args.platform, rev, ", ".join(possible_targets))) - sys.exit(1) + # pprint(possible_targets) Remove comment. ------------------------------ In bin/download-prebuilt-firmware.py <#97 (comment)> : > + + cache = load_cache() + if url in cache and (cache_ttl is None or ( + (datetime.now()-cache[url]['timestamp']).total_seconds() < + cache_ttl)): + data = cache[url]['data'] + else: + while True: + data = json.loads(urllib.request.urlopen(url).read().decode()) + if "message" in data: + print("Warning: {}".format(data["message"])) + time.sleep(1) + continue + else: + break + cache[url] = { This wrapping must be wrong?

Hmm? I'm guessing you mean that what gets read from the cache and what goes upstream isn't right? Or the cache[url] = should be somewhere else? I kinda hated writing this because it's hard to understand. If I had to do it over again I would find a package that has docs. I considered adding a --use-cache option, "to make it easier to understand" or something. that idea lasted about 30 seconds.

------------------------------ In bin/download-prebuilt-firmware.py <#97 (comment)> : > -def ls_github(url): - while True: - data = json.loads(urllib.request.urlopen(url).read().decode()) - if "message" in data: - print("Warning: {}".format(data["message"])) - time.sleep(1) - continue - return data + +class TargetNotFound(Exception): + pass + + +def ls_github(url, cache_ttl=None): + FIXME: Move cache to a class

meh. I'm sure someone has done this already. if any more work needs to be done, it is throw out my version and find a package.

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#97 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABauWXRFVVGb9muqs9moymeV-vM5BJgks5tSdWYgaJpZM4R6oZp> .

-- Carl K <blockquote><img src="https://assets-cdn.github.com/images/modules/open_graph/github-logo.png" width="48" align="right"><div><img src="https://assets-cdn.github.com/favicon.ico" height="14"> GitHub</div><div><strong><a href="https://github.com">Build software better, together</a></strong></div><div>GitHub is where people build software. More than 27 million people use GitHub to discover, fork, and contribute to over 77 million projects.</div></blockquote>

mithro

Please update the title of the pull request to start with a capital.

mithro · 2018-02-07T18:35:09Z

bin/download-prebuilt-firmware.py


    return possible_targets


+def find_last_rev(args, possible_revs):
+


Please remove all the extra newlines inside the def find_last_rev function.

mithro · 2018-02-07T18:36:36Z

bin/download-prebuilt-firmware.py

+                time.sleep(1)
+                continue
+            else:
+                break


Please format the cache[url] dictionary more normally. I think this is pep8 compliant?

cache[url] = { 'timestamp': datetime.now(), 'data': data, }

mithro · 2018-02-07T19:23:02Z

LGTM! -- Merging.

finds last rev that built, and caches github (indefinately, so kinda …

0346617

…bad.)

mithro reviewed Feb 6, 2018

View reviewed changes

bin/download-prebuilt-firmware.py Outdated

archive_url = get_url(args)

for rev in possible_revs:

Copy link

Member

mithro Feb 6, 2018

Choose a reason for hiding this comment

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

To many new lines....

fix cache problem: refresh possible_targets after 20 min.

43a288d

mithro reviewed Feb 6, 2018

View reviewed changes

CarlFK added 2 commits February 6, 2018 19:29

touchup formatting

d359f9b

make code robots happy

e2fbd80

timvideos deleted a comment Feb 7, 2018

mithro approved these changes Feb 7, 2018

View reviewed changes

comment touchups

a593107

CarlFK changed the title ~~finds last rev that built, and caches github (indefinately, so bad.)~~ finds last rev that built, and caches github Feb 7, 2018

timvideos deleted a comment Feb 7, 2018

mithro approved these changes Feb 7, 2018

View reviewed changes

formating touchup

eec4d88

CarlFK changed the title ~~finds last rev that built, and caches github~~ Finds last rev that built, and caches github Feb 7, 2018

mithro merged commit 7bd2e9f into timvideos:master Feb 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Finds last rev that built, and caches github #97

Finds last rev that built, and caches github #97

CarlFK commented Feb 6, 2018 •

edited

mithro Feb 6, 2018

mithro Feb 6, 2018

mithro Feb 6, 2018 •

edited

mithro commented Feb 6, 2018

mithro commented Feb 7, 2018

mithro left a comment

mithro Feb 7, 2018

CarlFK commented Feb 7, 2018 via email •

edited by unfurl-links bot

mithro left a comment

mithro Feb 7, 2018

mithro Feb 7, 2018

mithro commented Feb 7, 2018


		return possible_targets


		def find_last_rev(args, possible_revs):

Finds last rev that built, and caches github #97

Finds last rev that built, and caches github #97

Conversation

CarlFK commented Feb 6, 2018 • edited

mithro Feb 6, 2018

Choose a reason for hiding this comment

mithro Feb 6, 2018

Choose a reason for hiding this comment

mithro Feb 6, 2018 • edited

Choose a reason for hiding this comment

mithro commented Feb 6, 2018

mithro commented Feb 7, 2018

mithro left a comment

Choose a reason for hiding this comment

mithro Feb 7, 2018

Choose a reason for hiding this comment

CarlFK commented Feb 7, 2018 via email • edited by unfurl-links bot

mithro left a comment

Choose a reason for hiding this comment

mithro Feb 7, 2018

Choose a reason for hiding this comment

mithro Feb 7, 2018

Choose a reason for hiding this comment

mithro commented Feb 7, 2018

CarlFK commented Feb 6, 2018 •

edited

mithro Feb 6, 2018 •

edited

CarlFK commented Feb 7, 2018 via email •

edited by unfurl-links bot