Final Application Changes

[korydraughn]

I haven't verified the changes work. Will do that soon.

[korydraughn]

Ignoring clang-format for this project.

[korydraughn]

Existing tests pass with these changes.

The thing is, the tests only run if Nishant's change is reverted. The reason being that tip of main relies on changes in irods/irods which are only available post 4.3.0.

The question now becomes, do we want to target 4.3.0 instead of 4.3.1 for the final release of the REST API?

[trel]

README currently says:

This REST API requires an iRODS Server v4.2.0 or greater.

I’d rather not up the server requirement unless we need to for some good reason.

[alanking]

The thing is, the tests only run if Nishant's change is reverted. The reason being that tip of main relies on changes in irods/irods which are only available post 4.3.0.

The question now becomes, do we want to target 4.3.0 instead of 4.3.1 for the final release of the REST API?

I don't think it really matters what version of iRODS is running during the tests so long as the REST API can still communicate with 4.2 servers. Remember, it doesn't matter what version of the dev/runtime packages are required to build this. Am I missing something?

[korydraughn]

README currently says:

This REST API requires an iRODS Server v4.2.0 or greater.

I’d rather not up the server requirement unless we need to for some good reason.

Is that comment about communicating with a server or building against its dev package?

I'm talking about running the tests and compiling from source. I think the REST API requires a 4.3.0 dev package. And the tests will not work with a 4.3.1 environment because lib.py has a breaking change in it.

[trel]

Don’t care about the requirement to build.

I was asking about the server version this is standing in front of.

[korydraughn]

I don't think it really matters what version of iRODS is running during the tests so long as the REST API can still communicate with 4.2 servers. Remember, it doesn't matter what version of the dev/runtime packages are required to build this. Am I missing something?

That's correct. The issue is that Nishant's commit changes the python tests in a way that only works with the lib.py provided by what will be 4.3.1. If that commit isn't reverted, the tests cannot succeed.

So we can add something that allows the python tests to detect what type of server they are communicating with or we can state that the tests for this project require 4.3.0 test infrastructure.

[korydraughn]

Don’t care about the requirement to build.

I was asking about the server version this is standing in front of.

I think this can run against 4.2 servers, but I haven't confirmed that. Can look into that tomorrow though.

[trel]

Can the test framework run against multiple versions of the server? Understand if manual is enough. Just thinking out loud.

[korydraughn]

I believe the docker-based tests are configured/hard-coded to run against a 4.3.0 server. However, if someone ran it outside the docker environment, the tests could be run against any version of the server (assuming the tests don't make too many assumptions about where they're run or which version of the server they're talking to).

The one thing I know is the tests require python modules specific to 4.3.0.

[alanking]

So, running the tests requires 4.3.0 testing libraries, but cannot run because of the change which uses the 4.3.1(ish) interface for one testing library (as seen here: 0c8c799). If that commit is reverted, the tests pass.

The REST service is able to communicate with 4.2.x, 4.3.0, and 4.3.1(ish) servers and must be built using dev/runtime packages from 4.3.0.

Are these statements true? If so, I think this is fine. If not, please revise to be true so we are in agreement as to what is going on.

[korydraughn]

That's correct. The only thing I'm unsure about is 4.2. I think it will work just fine, but we need to confirm that before merging.

[korydraughn]

So far, the following operations pass manual testing via curl. The server was 4.2.11.

• /stream (read and write)
• /metadata
• /zonereport
• /query
• /ticket

The admin endpoint has an issue. It's not clear what it is yet.

[korydraughn]

Also, modifying the docker-based test harness and its config files for the REST API did not seem to work with a remote iRODS server.

I may give this another try soon.

[korydraughn]

Just tried pointing the test harness at the 4.2.11 server @alanking launched and the tests fail immediately.

I think it has to do with assumptions about where the REST API and iRODS server are running rather than there being an actual bug.

[korydraughn]

I was finally able to see the /admin endpoint work against a 4.2.11 server. I've also confirmed that the endpoints work against 4.2.12.

I'm going to revert Nishant's commit. Once that's done, this PR will be ready.

I have not tried pointing this at 4.3.1, but that's okay since users should move to the HTTP API for 4.3.1 communication.

All testing against 4.2 was carried out using bash scripts I wrote (i.e. I used curl).

Any remaining thoughts on this PR before I squash/pound?

[alanking]

I say let's do it. Please await @trel's approval, though. This is the final work to be done in here, so want to make sure.

[trel]

Seems good to me. Let’s get it in.

[korydraughn]

Squashed and pounded.