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

Rest: Filter buildsets by sourcestamp attributes #6518

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jangmarker
Copy link
Contributor

Before implementing unit tests I'd first like to confirm that this approach can go forward. Open questions that I need feedback on:

  • Is it ok to encode the sourcestamp objects as JSON in the REST query?
  • Is the SQL query approach reasonable?

Description

Allow the buildsets REST endpoint to filter by sourcestamp attributes.
We use the 'contains' relation to filter by a set of sourcestamp attributes:
buildsets?sourcestamps__contains={"ssid":26}

The list of buildsets is then only listing buildsets that have a
sourcetsamp with that ssid. If the sourcestamps__contains filter is used
multiple times it follows the "or" semantics like the other filters. If
a single filter lists multiple attributes these follow and semantics,
within the filter.

The filters are implemented on SQL-level. Necessary joins are only
performed if this filter is actually used.

Contributor Checklist:

  • I have updated the unit tests
  • I have created a file in the newsfragments directory (and read the README.txt in that directory)
  • I have updated the appropriate documentation

@jangmarker jangmarker force-pushed the filter-buildset-by-sourcestamp branch from c987817 to 6d1242b Compare May 14, 2022 17:32
@codecov
Copy link

codecov bot commented May 14, 2022

Codecov Report

Merging #6518 (dc4ae9c) into master (145a781) will decrease coverage by 0.04%.
The diff coverage is 48.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6518      +/-   ##
==========================================
- Coverage   92.08%   92.04%   -0.05%     
==========================================
  Files         343      343              
  Lines       38636    38660      +24     
==========================================
+ Hits        35578    35583       +5     
- Misses       3058     3077      +19     
Impacted Files Coverage Δ
master/buildbot/data/sourcestamps.py 89.36% <37.50%> (-10.64%) ⬇️
master/buildbot/db/buildsets.py 90.71% <52.94%> (-5.26%) ⬇️
master/buildbot/util/queue.py 90.69% <0.00%> (-6.98%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 145a781...dc4ae9c. Read the comment docs.

Allow the buildsets REST endpoint to filter by sourcestamp attributes.
We use the 'contains' relation to filter by a set of sourcestamp attributes:
buildsets?sourcestamps__contains={"ssid":26}

The list of buildsets is then only listing buildsets that have a
sourcestamp with that ssid. If the sourcestamps__contains filter is used
multiple times it follows the "or" semantics like the other filters. If
a single filter lists multiple attributes these follow and semantics,
within the filter.

sourcestamps are encoded as JSON in the REST query.

The filters are implemented on SQL-level. Necessary joins are only
performed if this filter is actually used.
@jangmarker jangmarker force-pushed the filter-buildset-by-sourcestamp branch from 6d1242b to dc4ae9c Compare May 17, 2022 17:30
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.

None yet

1 participant