Skip to content

Conversation

@d-w-moore
Copy link
Collaborator

In other words, setting opt2 = val2 does not reset opt1 back to its default value.

Copy link
Contributor

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.

@d-w-moore d-w-moore changed the title [_709] preserve options in chaining obj.metadata(opt1=val1)(opt2=val2) [#709] preserve options in chaining obj.metadata(opt1=val1)(opt2=val2) May 6, 2025
@d-w-moore d-w-moore marked this pull request as draft November 27, 2025 04:44
…val2)

In other words, setting opt2 = val2 does not reset opt1 back to its default value.

[_709] correct and streamline. _opts and __kw should be separate.

[_709] reasonable copy of _opts
@d-w-moore
Copy link
Collaborator Author

Evidently it is still possible to fail here:

https://github.com/d-w-moore/python-irodsclient/blob/388ad3d858ebcc7be981d1c065a4dd57ffc066cf/irods/test/data_obj_test.py#L3317

The modify time turned out on this occasion to be one second later than the access time on a just-created replica open for write:


======================================================================
FAIL [0.216s]: test_access_time__issue_700 (data_obj_test.TestDataObjOps.test_access_time__issue_700)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/prc.rw/irods/test/data_obj_test.py", line 3317, in test_access_time__issue_700
    self.assertEqual(data.access_time, data.modify_time)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: datetime.datetime(2025, 11, 29, 4, 12, 23, tzinfo=datetime.timezone.utc) != datetime.datetime(2025, 11, 29, 4, 12, 24, tzinfo=datetime.timezone.utc)

----------------------------------------------------------------------
Ran 302 tests in 319.841s

FAILED (failures=1, skipped=16)

@korydraughn any ideas why this might occasionally happen?

@d-w-moore d-w-moore marked this pull request as ready for review November 29, 2025 04:31
@d-w-moore
Copy link
Collaborator Author

Squashed , ready for review.

Copy link
Contributor

@alanking alanking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am especially interested in the changes related to #768 given the milestone for that issue.

@korydraughn
Copy link
Contributor

korydraughn commented Dec 1, 2025

Evidently it is still possible to fail here:

https://github.com/d-w-moore/python-irodsclient/blob/388ad3d858ebcc7be981d1c065a4dd57ffc066cf/irods/test/data_obj_test.py#L3317

The modify time turned out on this occasion to be one second later than the access time on a just-created replica open for write:


======================================================================
FAIL [0.216s]: test_access_time__issue_700 (data_obj_test.TestDataObjOps.test_access_time__issue_700)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/prc.rw/irods/test/data_obj_test.py", line 3317, in test_access_time__issue_700
    self.assertEqual(data.access_time, data.modify_time)
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: datetime.datetime(2025, 11, 29, 4, 12, 23, tzinfo=datetime.timezone.utc) != datetime.datetime(2025, 11, 29, 4, 12, 24, tzinfo=datetime.timezone.utc)

----------------------------------------------------------------------
Ran 302 tests in 319.841s

FAILED (failures=1, skipped=16)

@korydraughn any ideas why this might occasionally happen.

Hmm, I don't think the python test code is equivalent to the following C++ test code?

https://github.com/irods/irods/blob/1a5670d157ce932cc2888d74bb29c831e2db53ae/unit_tests/src/test_rc_update_replica_access_time.cpp#L312-L323

https://github.com/irods/irods/blob/1a5670d157ce932cc2888d74bb29c831e2db53ae/unit_tests/src/test_rc_update_replica_access_time.cpp#L689-L706

The C++ test code I linked to expects the replica to be in the intermediate state.

I think the problem with the python test is that the following call closes the data object, resulting in the close operation updating the mtime.

data = self.sess.data_objects.create(

What you need to do is open (and create) a new replica and do your checks while the replica is open. Closing the replica is what causes the atime and mtime to desync.

@d-w-moore
Copy link
Collaborator Author

What you need to do is open (and create) a new replica and do your checks while the replica is open. Closing the replica is what causes the atime and mtime to desync.

Ah, you're right. I see. Will try it a different way.

@korydraughn
Copy link
Contributor

What's the status of this PR?

@d-w-moore
Copy link
Collaborator Author

What's the status of this PR?

Looks like we are good. Need a squash, and perhaps a codacy review.

Copy link
Contributor

@korydraughn korydraughn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall.

Just a few comments on the README.

increase code efficiency if for example a lot of AVUs must be added or deleted
at once without reading any back again.

```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
```
```py

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this was missed.

[_768] add test

[_768] reasonable handling of _meta

reload test altered for clarity
Example: iRODSBinOrStringMeta allows storing arbitrary octet strings in metadata.
d-w-moore and others added 2 commits December 8, 2025 15:16
We test a still open replica at its point of creation, asserting mod and access times are equal.

And we clean up after ourselves now.
Co-authored-by: Kory Draughn <korydraughn@ymail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants