Skip to content
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

stagingapi: Avoid search/package query to determine devel project. #654

Merged
merged 2 commits into from Feb 6, 2017

Conversation

@jberry-suse
Copy link
Contributor

jberry-suse commented Jan 26, 2017

The search/package query is typically very slow and can spend several minutes timing out. The concept of loading all package meta data at once is attractive, but given that the query regularly increases the runtime of the script by an order of magnitude or more making individual calls seems advantageous. These calls are already included in the cache which means the initial request avoids the 10+ minute wait and repetitive calls have no additional cost.

I ran into this query failing >5 times today and have been considering this change for a while. In the check_source.py port I changed to query for individual packages since it should requests as they come and would not see much benefit from loading all package data at once. See ReviewBot.get_devel_project().

I see factory averages >50 packages in backlog which will be roughly the number of calls made during a list call. Leap 42.3 had ~25 packages in backlog when I took a look. I ran staging commands using this change and worked as expected. Definitely a trade off, but I think in general this will be far more consistent (generally does not timeout) and relatively quick response per query.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jan 26, 2017

Coverage Status

Coverage increased (+0.2%) to 46.5% when pulling e37b46c on jberry-suse:avoid-search-package into 53a969e on openSUSE:master.

node = ET.fromstring(''.join(m)).find('devel')
if node is not None:
return node.get('project')
except urllib2.HTTPError:

This comment has been minimized.

Copy link
@lnussel

lnussel Jan 30, 2017

Member

shouldn't this catch 404 only?

This comment has been minimized.

Copy link
@jberry-suse

jberry-suse Feb 1, 2017

Author Contributor

makes sense, changed.

@lnussel

This comment has been minimized.

Copy link
Member

lnussel commented Jan 30, 2017

I think I made this optimization for SP2 where we had hundreds of packages in backlog. I hope we won't get into this situation again although KDE updates are also quite big :-) Also, I think the adi command calls get_devel_project in the wrong way. It calls it on source project which doesn't make sense.

jberry-suse added 2 commits Jan 26, 2017
The search/package query is typically very slow and can spend several
minutes timing out. The concept of loading all package meta data at once
is attractive, but given that the query regularly increases the runtime
of the script by an order of magnitude or more making individual calls
seems advantageous. These calls are already included in the cache which
means the initial request avoids the 10+ minute wait and repetitive calls
have no additional cost.
@jberry-suse jberry-suse force-pushed the jberry-suse:avoid-search-package branch from e37b46c to 60559a1 Feb 1, 2017
@coveralls

This comment has been minimized.

Copy link

coveralls commented Feb 1, 2017

Coverage Status

Coverage increased (+0.2%) to 45.443% when pulling 60559a1 on jberry-suse:avoid-search-package into d08d430 on openSUSE:master.

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Feb 1, 2017

  • created #662 to update the adi based logic
  • updated to check for 404
  • added second commit to update ReviewBot same function

Potentially could keep the search code around use anytime number of requests exceeds a certain threshold? Also not sure what sort of value vs amount of work it would take to allow ReviewBot and stagingapi to share more code.

@lnussel lnussel merged commit a8c2f78 into openSUSE:master Feb 6, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jberry-suse jberry-suse deleted the jberry-suse:avoid-search-package branch Feb 9, 2017
@jberry-suse jberry-suse added this to performance in Automate staging workflow Mar 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.