FIREFLY-1331: Add Version validation for compatibility check between python client and firefly server by jaladh-singhal · Pull Request #79 · Caltech-IPAC/firefly_client

jaladh-singhal · 2026-04-10T01:11:37Z

Added server compatibility check on FireflyClient initialization — raises ValueError if the server version is below the required minimum, warns and proceeds if the version endpoint is unreachable
Added _server_compat.py module with version parsing logic, which also handles Firefly's non-standard version strings (DEV with branch/commit suffix, PRE releases)
Added unit tests (tests/test_server_compat.py) and a dev test notebook (examples/development_tests/test-version.ipynb)

Also see Firefly PR: Caltech-IPAC/firefly#1936

Extra:

Introduced unit tests to this package for the first time: Added pytest as an optional [tests] dependency; updated developer docs with instructions to run tests
Updated url_cmd_service from sticky/CmdSrv to CmdSrv/sync which seems more modern

Testing

Check the test notebook (examples/development_tests/test-version.ipynb): nb-review's rendered version in the following comment.
Check the unit test file (tests/test_server_compat.py) to understand how version parsing and compatiblity check logic is working for different cases
[Optional] Run the unit tests and test notebook locally by checking out this branch and then following instructions in development docs:

- Introduced _server_compat.py for managing server version compatibility. - Implemented version retrieval and compatibility checking in FireflyClient. - Updated pyproject.toml to include packaging dependency.

… validation logic

And update documentation and pyproject.toml for pytest

review-notebook-app · 2026-04-10T01:11:44Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

jaladh-singhal · 2026-04-10T01:35:48Z


 [project.optional-dependencies]
+tests = [
+    "pytest",


this is an optional dependency for tests so won't impact end user.

jaladh-singhal · 2026-04-10T01:39:34Z

    "websocket-client",
-    "requests"
+    "requests",
+    "packaging",


packaging is one of the core utilities developed and maintained by python packaging authority (pypa): https://github.com/pypa/packaging. It's also used by pip itself internally so in most cases, it is already installed in user's python environment.

robyww · 2026-04-10T19:28:24Z

+def is_server_compatible(server_version: str|None) -> bool:
+    if not server_version:
+        return False
+    version = standardize_version(server_version)
+    return version is not None and version >= _MIN_VERSION


I am concerned that this might not be the best way to validate the version. This look like just a string compare. I think you need to separate the major and the minor the do something similar to BrowserInfo.js. That way you can ignore the type of build.

https://github.com/Caltech-IPAC/firefly/blob/252b29119973f6ac0ad1fa0e38ed17e92cfc91d1/src/firefly/js/util/BrowserInfo.js#L118-L125

It's not a string compare, it's comparing two packaging.Version objects (standardize_version converts firefly version string to Version object) which knows how to handle relational operators.

You can look at the unit tests to confirm if it's missing any of the cases you are concerned about: https://github.com/Caltech-IPAC/firefly_client/blob/FIREFLY-1331-version-validation/tests/test_server_compat.py#L31-L49

ok, I am going to probably need an overview of how this works. Maybe we can do that on Monday.

You can read each test case as: ver >= min_ver = bool. So:

_FIXED_MIN_VERSION = standardize_version('2026.1-DEV') ('2026.1-PRE-3', True), # pre-release of same version cycle ('2026.1', True), # formal release >= minimum

becomes:

'2026.1-PRE-3' >= '2026.1-DEV' = True '2026.1' >= '2026.1-DEV' = True

and so on

Yes, I can demo it on Monday too.

robyww

Overall I think code is very clean and works well. I have several comments and would like some small changes.

Changes

treat reachable : false as 2025.5 and test accordingly
only compare with major and minor. Ignore revision or build type (dev, pre), i.e. a 2026.1 pre should be treated as 2026.1
set the MIN_SERVER_VERSION to a truly minimally compatible version.

Other comments

Bringing the another packages seem like a little overkill. More packages, even if they are commonly used, always add some complexity. Maybe a package would be helpful is our comparison was more complex but I don't think it needs to be.
I think the comparison could have been a few lines of code in fc_utils.py. I am not sure having a _server_compat.py is necessary.
I am glad you implemented the test

robyww · 2026-04-17T15:37:19Z

+        self.url_cmd_service = urljoin(f'{protocol}://{self.location}/', 'CmdSrv/sync')
+        self.url_browser = urljoin(urljoin(f'{protocol}://{self.location}/', html_file), '?__wsch=')


good cleanup

robyww · 2026-04-17T16:10:03Z

+        server_response = self.session.get(version_url, headers=self.header_from_ws)
+
+        reachable = server_response.status_code == 200
+        server_version = get_server_version(server_response.json()) if reachable else None


I think it would be better to return a dic

'reachable': False

to keep the response consistent.

robyww · 2026-04-17T16:17:08Z

+
+        ver = self._confirm_version()
+        if not ver['reachable']:
+            warn(f'Could not retrieve version of the Firefly server {url}. Proceeding without compatibility check.')


We are going to get this a lot at first. i think we need to include this in our cases.
Not reachable is 2025.5 or earlier. I think when it is not reachable you need to assign the server version to be 2025.5 and do the computation like that. If FireflyClient requires 2026.1 then it should be marked as not compatable otherwise is should be compatable.

robyww · 2026-04-17T16:31:54Z

+# Minimum version of Firefly server that this version of firefly_client is compatible with
+# Must be updated when a new release of firefly_client is made that relies on updates in Firefly server.
+# For DEV version of Firefly server, the branch and commit info isn't considered in comparison so can be omitted.
+MIN_SERVER_VERSION = '2026.1-DEV' # minimum bound on DEV includes PRE and formal releases


unless you have could reason, it probably should be set to what ever is truly minimally compatable. 2025.5? 2025.4?

robyww · 2026-04-17T19:09:29Z

I just found a significant problem. You might want to correct this on the service call. It is a little bit tricky.

They problem is this. Version is the version of the application not the library. Firefly Library Version is the version of firefly but it is not included with the firefly app. When you really want to check is Firefly Library Version and have it always included. However, I think the key is a little awkward. You can change it because it is only referenced in iterators.

Firefly app

https://fireflydev.ipac.caltech.edu/firefly/CmdSrv/sync?cmd=CmdVersion

{
  "Version": "2026.1-DEV:HEAD_2982",
  "Built On": "Fri Apr 17 07:02:49 UTC 2026",
  "Git commit": "2982d59e8"
}

Rubin Portal

http://localhost:8080/suit/CmdSrv/sync?cmd=CmdVersion

{
  "Version": "v2026.1",
  "Built On": "Fri Apr 17 12:42:11 MDT 2026",
  "Git commit": "96dd01c",
  "Firefly Library Version": "2026.1-DEV_2982",
  "Firefly Git Commit": "2982d59e8"
}

IrsaViewer

https://irsadev.ipac.caltech.edu/irsaviewer/CmdSrv/sync?cmd=CmdVersion

{
  "Version": "v1.0",
  "Built On": "Fri Apr 17 07:07:55 UTC 2026",
  "Git commit": "cfe6ade6",
  "Firefly Library Version": "2026.1-DEV:HEAD_2982",
  "Firefly Git Commit": "2982d59e8"
}

My suggestion (java side) is to modify VersionUtil.getVersionInfo() to do the following

versionInfo.add(new KeyVal<>("Version", getFireflyVersionStr(v)));
versionInfo.add(new KeyVal<>("Application Version", getVersionStr(v)));

Both keys should always be added.

Then take out line 139-140

        if (v.getMajor()>0)
            versionInfo.add(new KeyVal<>("Firefly Library Version", getFireflyVersionStr(v)));

- Switch to new endpoint payload shape: success flag + data['Firefly Version'] - Unknown/unreachable versions pass through as compatible for backward compat - Set MIN_SERVER_VERSION to '2025.4'; add dependency log to guide future bumps - Update release procedure: only bump on API-breaking server changes Co-authored-by: Copilot <copilot@github.com>

Co-authored-by: Copilot <copilot@github.com>

jaladh-singhal · 2026-04-29T22:18:15Z

@robyww I updated PR to address following:

treat reachable : false as 2025.5 and test accordingly

If version endpoint is not reachable, the server version is now returned as None which now falls back to compatible: True.

only compare with major and minor. Ignore revision or build type (dev, pre), i.e. a 2026.1 pre should be treated as 2026.1

Done. Removed packaging package - I'm directly comparing tuple of int now (major, minor).

set the MIN_SERVER_VERSION to a truly minimally compatible version.

I digged through past release notes and PR logs and found firefly_client PRs that depend on firefly PR. I added them to comment where MIN_SERVER_VERSION is defined and explained that we only need firefly release that breaks API of firefly_client. This PR doesn't and previous one also doesn't but the one before does which was released as 2025.4 so that's the new and true MIN_SERVER_VERSION

robyww

Looks good.

jaladh-singhal added 4 commits April 3, 2026 17:55

FIREFLY-1331: Add server compatibility checks and version validation

de47c8c

- Introduced _server_compat.py for managing server version compatibility. - Implemented version retrieval and compatibility checking in FireflyClient. - Updated pyproject.toml to include packaging dependency.

FIREFLY-1331: Refactor server compatibility checks and update version…

94b2e4f

… validation logic

FIREFLY-1331: Update release procedure to clarify version bumping steps

68da277

Add unit tests and notebook for version compatibility

0796869

And update documentation and pyproject.toml for pytest

jaladh-singhal requested review from loitly and robyww April 10, 2026 01:11

jaladh-singhal self-assigned this Apr 10, 2026

jaladh-singhal added the enhancement label Apr 10, 2026

jaladh-singhal changed the title ~~Firefly 1331 version validation~~ FIREFLY-1331: Add Version validation for compatibility check between python client and firefly server Apr 10, 2026

jaladh-singhal mentioned this pull request Apr 10, 2026

FIREFLY-1331: Add Version validation for compatibility check between python client and firefly server Caltech-IPAC/firefly#1936

Merged

jaladh-singhal commented Apr 10, 2026

View reviewed changes

robyww reviewed Apr 10, 2026

View reviewed changes

robyww requested changes Apr 17, 2026

View reviewed changes

jaladh-singhal and others added 3 commits April 29, 2026 14:03

FIREFLY-1331: Only do major and minor check for compatiblity

bae21c2

Co-authored-by: Copilot <copilot@github.com>

FIREFLY-1331: Update test-version.ipynb

cefc418

jaladh-singhal requested a review from robyww April 29, 2026 22:11

robyww approved these changes Apr 29, 2026

View reviewed changes

jaladh-singhal merged commit 5ca754c into master Apr 29, 2026
1 check passed

		self.url_cmd_service = urljoin(f'{protocol}://{self.location}/', 'CmdSrv/sync')
		self.url_browser = urljoin(urljoin(f'{protocol}://{self.location}/', html_file), '?__wsch=')

Conversation

jaladh-singhal commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Extra:

Testing

Uh oh!

review-notebook-app Bot commented Apr 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robyww left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robyww commented Apr 17, 2026

Firefly app

Rubin Portal

IrsaViewer

Uh oh!

jaladh-singhal commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robyww left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jaladh-singhal commented Apr 10, 2026 •

edited

Loading

jaladh-singhal commented Apr 29, 2026 •

edited

Loading