Skip to content

Catch sdist exceptions#1

Open
caspervdw wants to merge 2 commits intomasterfrom
casper-catchsdisterror
Open

Catch sdist exceptions#1
caspervdw wants to merge 2 commits intomasterfrom
casper-catchsdisterror

Conversation

@caspervdw
Copy link

No description provided.

@caspervdw caspervdw requested a review from reinout August 23, 2018 12:45
Copy link
Collaborator

@reinout reinout left a comment

Choose a reason for hiding this comment

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

Het is een beetje een vraag wat je wilt. Zo'n logger.error() berichtje verdwijnt waarschijnlijk gewoon in het digitale archief (want we hebben nooit goed geregeld dat de error ergens terechtkomt waat 'ie gelezen wordt).

Dus: prima, maar in de praktijk wel even oppassen.

(Qua N&S gebruik: ik weet niet meer of ik de checkoutmanager config opgeschoond had? lizard-ui en zo eruit, dat wordt toch niet meer gebruikt. Opschonen zou al veel fouten van oude pakketten kunnen voorkomen: misschien dat er dan een eenvoudige zabbix mededeling aan gekoppeld kan worden. Of het even in sentry hangen, dat is misschien nog netter.)

logger.debug(command("%s setup.py sdist" % python))
except SdistCreationError:
logger.error("Sdist exception while building %s" % tag)
return
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hier return je None terwijl er normaliter een path naar een tarball wordt teruggegeven. Gaat dat goed?

Copy link
Author

Choose a reason for hiding this comment

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

Ja zie hier:

if tarball is None:

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ik ken m'n eigen code niet meer :-)

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.

2 participants