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

API inconsistency of the GET /source/{prj_name} route with deleted=0 parameter showing multibuild packages #16911

Open
dcermak opened this issue Oct 2, 2024 · 15 comments · May be fixed by #16955
Assignees
Labels
Bug Frontend Things related to the OBS RoR app P2 If possible, assign this to yourself and fix it ASAP

Comments

@dcermak
Copy link
Contributor

dcermak commented Oct 2, 2024

Issue Description

The GET /source/{prj_name} is inconsistent when it comes to the deleted=0 parameter. Omitting the parameter lists only source packages but adding it includes multibuild flavors

Expected Result

The output of GET /source/{prj_name} and GET /source/{prj_name}?deleted=0 should be the same.

How to Reproduce

$ osc api '/source/devel:microos'|grep ":"|wc -l
0
$ osc api '/source/devel:microos?deleted=0'|grep ":"|wc -l
2

Additional Info

This appears to impact interconnect in some way as well:

$ osc api '/source/devel:BCI:SLE-15-SP6'|grep ":"|wc -l
0

while there are multibuild flavors via interconnect on IBS:

$ isc api '/source/openSUSE.org:devel:BCI:SLE-15-SP6'|grep ":"|wc -l
2
@hennevogel hennevogel added Bug Frontend Things related to the OBS RoR app labels Oct 2, 2024
@hennevogel
Copy link
Member

Does this have some impact for something you do?

@dcermak
Copy link
Contributor Author

dcermak commented Oct 2, 2024

Does this have some impact for something you do?

yes, especially the last one is annoying as I get different results over interconnect

@hennevogel hennevogel added the P2 If possible, assign this to yourself and fix it ASAP label Oct 2, 2024
@eduardoj
Copy link
Member

eduardoj commented Oct 2, 2024

Related to already closed #9715.

danidoni added a commit to danidoni/open-build-service that referenced this issue Oct 2, 2024
This way `?delete=0` and `?` behaves consistently, and only `?delete=1`
activates the behaviour for deleted projects

Fixes openSUSE#16911
@adrianschroeter
Copy link
Member

delete=0 is currently used as workaround to get the backend answer, so we can not do this.

Please just don't send the delete query parameter if you don't want the multibuild containers.

danidoni added a commit to danidoni/open-build-service that referenced this issue Oct 2, 2024
This way `?deleted=0` and `?` behaves consistently, and only `?deleted=1`
activates the behaviour for deleted projects

Fixes openSUSE#16911
@adrianschroeter
Copy link
Member

and use just ?deleted if you want it. This is the meanwhile documented way, there is actually no reason to add a value to it.

However, since some tools need to by-pass the api, because they require the backend informations (eg. for originpackage information), we can not become incompatible here.

@dcermak
Copy link
Contributor Author

dcermak commented Oct 2, 2024

delete=0 is currently used as workaround to get the backend answer, so we can not do this.

Wouldn't it make more sense to use a private API for this?

Please just don't send the delete query parameter if you don't want the multibuild containers.

I am not doing this, but something in the interconnect code does that for me.

Also, osc ls sends deleted=0 by default (jfyi @dmach), that's how I found this

danidoni added a commit to danidoni/open-build-service that referenced this issue Oct 3, 2024
There was a consistency gap between sources coming from local projects
and sources coming from remote projects. Multibuild flavors were only
shown for remote projects. Now the multibuild flavors are shown in both
cases.

Fixes openSUSE#16911
danidoni added a commit to danidoni/open-build-service that referenced this issue Oct 3, 2024
There was a consistency gap between sources coming from local projects
and sources coming from remote projects. Multibuild flavors were only
shown for remote projects. Now the multibuild flavors are shown in both
cases.

Fixes openSUSE#16911
@eduardoj
Copy link
Member

eduardoj commented Oct 7, 2024

and use just ?deleted if you want it. This is the meanwhile documented way, there is actually no reason to add a value to it.

However, since some tools need to by-pass the api, because they require the backend informations (eg. for originpackage information), we can not become incompatible here.

The documentation states for the deleted parameter: Set to 1 to list the packages of a deleted project.

It is not explicitly defined what happens if passing an empty deleted parameter, or deleted=0... Nothing I would change from the documentation part.

But I would expect some consistency on the result of the endpoint in all the cases where we don't pass deleted=1.

@danidoni
Copy link
Contributor

danidoni commented Oct 7, 2024

Adrian,

delete=0 is currently used as workaround to get the backend answer, so we can not do this.

Where is this being used? Can you point me to it?

@hennevogel
Copy link
Member

Guess another scmsync thing?

➜  ~ osc -A https://api-test.opensuse.org api '/source/home:hennevogel:scmsync-project'
<directory count="0"/>
➜  ~ osc -A https://api-test.opensuse.org api '/source/home:hennevogel:scmsync-project?deleted=0'
<directory>
  <entry name="ctris"/>
  <entry name="ponysay"/>
  <entry name="webhook"/>
</directory>

@dcermak
Copy link
Contributor Author

dcermak commented Oct 8, 2024 via email

@hennevogel
Copy link
Member

No, scmsync is involved here,

I was talking about where this is used...

@adrianschroeter
Copy link
Member

adrianschroeter commented Oct 8, 2024

again, deleted=0 is unfortunately an expected workaround of many tools (including osc) to bypass the api answer. There are many reasons to do so, as the api answer is incomplete in many situations. multibuild is one thing, project link is another one, scmsync yet another, remote projects one more and I may have forgotten more situations. And yes, you can even combine these.

We accept this inconsistency, as for many usecases the simple api (aka database) answer is good enough. And there is no reason to break these implementation and cause plenty of problems in many client tools.

However, if you are not aware of the differences of "deleted=0", just use the documented api way by using it either as boolean "?deleted&..." or don't use the deleted argument.

So where is the problem at all? Having two different answers based on the cgi parameters is absolute okay IMHO.

@dcermak
Copy link
Contributor Author

dcermak commented Oct 8, 2024 via email

@adrianschroeter
Copy link
Member

well, you can't atm, but couldn't you just ignore the additional information?

Or you could always call with "deleted=0", so you would get a consistant behaviour, if this is more important.

danidoni added a commit to danidoni/open-build-service that referenced this issue Oct 8, 2024
There was a consistency gap between sources coming from local projects
and sources coming from remote projects. Multibuild flavors were only
shown for remote projects. Now the multibuild flavors are shown in both
cases.

Fixes openSUSE#16911
danidoni added a commit to danidoni/open-build-service that referenced this issue Oct 8, 2024
There was a consistency gap between sources coming from local projects
and sources coming from remote projects. Multibuild flavors were only
shown for remote projects. Now the multibuild flavors are shown in both
cases.

Fixes openSUSE#16911
@adrianschroeter
Copy link
Member

if consistency is really that important here, we may consider always deliver the backend answer for package listings and never generate it based on database data ....

danidoni added a commit to danidoni/open-build-service that referenced this issue Oct 9, 2024
There was a consistency gap between sources coming from local projects
and sources coming from remote projects. Multibuild flavors were only
shown for remote projects. Now the multibuild flavors are shown in both
cases.

Fixes openSUSE#16911
@krauselukas krauselukas self-assigned this Oct 10, 2024
krauselukas added a commit to krauselukas/open-build-service that referenced this issue Oct 14, 2024
Right now we use the presence of the `deleted` query parameter
as a workaround in order to force the request being passed
directly to the backend and returning results based on the
backend data over returning results based on frontend database data.

This workaround currently leads to inconsistant results being returned
by the API.

Instead of using this workaround we better should always
let the backend handle the request and return the data based on
it's info.

Fixes openSUSE#16911
krauselukas added a commit to krauselukas/open-build-service that referenced this issue Oct 15, 2024
Right now we use the presence of the `?deleted=0` query parameter
as a workaround to force the request being passed directly to the
backend and returning results based on backend data over returning
results based on frontend database entries.

This workaround currently leads to inconsistant results being returned
by the API.

Instead of using this workaround we better should always
let the backend handle the request (where possible) and return
the data based on it's info.

Fixes openSUSE#16911
krauselukas added a commit to krauselukas/open-build-service that referenced this issue Oct 16, 2024
Right now we use the presence of the `?deleted=0` query parameter
as a workaround to force the request being passed directly to the
backend and returning results based on backend data over returning
results based on frontend database entries.

This workaround currently leads to inconsistant results being returned
by the API.

Instead of using this workaround we better should always
let the backend handle the request (where possible) and return
the data based on it's info.

Fixes openSUSE#16911
krauselukas added a commit to krauselukas/open-build-service that referenced this issue Oct 23, 2024
Right now we use the presence of the `?deleted=0` query parameter
as a workaround to force the request being passed directly to the
backend and returning results based on backend data over returning
results based on frontend database entries.

This workaround currently leads to inconsistant results being returned
by the API.

Instead of using this workaround we better should always
let the backend handle the request (where possible) and return
the data based on it's info.

Fixes openSUSE#16911
@krauselukas krauselukas assigned ncounter and unassigned krauselukas Oct 28, 2024
danidoni added a commit to danidoni/open-build-service that referenced this issue Dec 16, 2024
We do this to be consistent with the behaviour showed when passing a
remote project.

Fixes openSUSE#16911
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Frontend Things related to the OBS RoR app P2 If possible, assign this to yourself and fix it ASAP
Projects
None yet
7 participants