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

Make the client send its version when calling the server #258

Closed
chaen opened this issue Jun 13, 2024 · 5 comments · Fixed by #267
Closed

Make the client send its version when calling the server #258

chaen opened this issue Jun 13, 2024 · 5 comments · Fixed by #267
Assignees
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@chaen
Copy link
Contributor

chaen commented Jun 13, 2024

The server should given an error if the client version is too old

The minimum version should be configured in the pyproject as a metadata such that the extension can overwrite it (diracx.min_client_version)

@chaen chaen added good first issue Good for newcomers help wanted Extra attention is needed labels Jun 13, 2024
@chrisburr
Copy link
Member

chrisburr commented Jun 21, 2024

  • diracx-client/src/diracx/client/_patch.py
X-DIRACX-CLIENT-VERSION: 1.2.4
[project.entry-points."diracx.min_client_version"]
diracx = "1.2.3"
lhcbdirac = "5.6.7"

@chaen
Copy link
Contributor Author

chaen commented Jul 12, 2024

Side note: I don't think we will have millions of them, but do we want the diracx headers to be easily distinguishable from others ? That is, do we want e.g. DiracX-Client-Version more than client-version ?
It's nice when it's easy to spot so I would be tempted to add an extra prefix. The X- standard is deprecated though (https://www.rfc-editor.org/rfc/rfc6648#page-4)

Also, this raises the question: what do we do in case there isn't a version ? (swagger, diracx-web)

@natthan-pigoux
Copy link
Contributor

Yes, I already changed the name with X-Diracx-Client-Version. I will remove the X- then.
I also added a TODO with the question on what to do in that cas.
Thanks again for your help @chaen :)

@aldbr
Copy link
Contributor

aldbr commented Jul 12, 2024

what do we do in case there isn't a version ? (swagger, diracx-web)

I guess the server should check the DiracX-Client-Version only if it is provided by the client, shouldn't it?

@natthan-pigoux
Copy link
Contributor

Another point is that the entrypoints in the toml for the "min_client_version" cannot be a static string but:
[project.entry-points."diracx.min_client_version"]
diracx = "diracx.routers:DIRACX_MIN_CLIENT_VERSION"

I hard coded it there for now: d0ff92f#diff-424d2a991980de5065c3b1e58f5066bf78641a4804b34bb5977be2ad95289ae7R483

@fstagni fstagni linked a pull request Oct 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants