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

Feature request: Expose lastUpdated in .stat() #380

Closed
OddBloke opened this issue Sep 23, 2022 · 10 comments
Closed

Feature request: Expose lastUpdated in .stat() #380

OddBloke opened this issue Sep 23, 2022 · 10 comments
Labels
Help Wanted We will be glad if somebody proposes a solution via PR

Comments

@OddBloke
Copy link
Contributor

Artifactory does not perform deb metadata update synchronously: we've run into errors in automation due to this, and so we now have some logic which waits for the metadata we expect to be present. Having the lastUpdated timestamp available to us in .stat()s return would make this logic easier to implement: currently we directly call artifact._accessor.get_stat_json(artifact) to get access to it.

(Thanks again for this library: it continues to work really well for us!)

@OddBloke
Copy link
Contributor Author

One potential solution to this, and a whole class of related requests, would be to store the JSON returned by Artifactory on the ArtifactoryFileStat object returned: then our code could do (e.g.) artifact.stat().json['lastUpdated']. (I could contribute this change if you'd like!)

@allburov
Copy link
Member

Hi!

It'd be better if stat function accepts the new raw parameter (False by default) and if it's set - save json response to _json field in ArtifactoryFileStat (None by default) and we can get access to original json by calling json() function.

So to access lastUpdated you'll need to run:

stat: ArtifactoryFileStat = artifact.stat(raw=True) 
raw_data: Dict = stat.json()
print(raw_data['lastUpdated']) 

I could contribute this change if you'd like
It would be great!

@allburov allburov added the Help Wanted We will be glad if somebody proposes a solution via PR label Sep 26, 2022
@beliaev-maksim
Copy link
Member

currently stats has lastModified, do you mean artifactory API provides additional output?

mtime=dateutil.parser.parse(jsn["lastModified"]),

if yes, then why just not to include it the same way, what will be the purpose of getting raw data here?

@OddBloke
Copy link
Contributor Author

Good point! I had thought that ArtifactoryFileStat was mimicking the return type of a stdlib stat interface (and so I didn't want to add another similar field to it), but looking again that isn't true.

utime doesn't feel quite right as a field name, I think the name collision with https://docs.python.org/3/library/os.html#os.utime would cause some confusion. How would we feel about following the Artifactory API and using last_updated (so my code would do artifact.stat().last_updated)?

@beliaev-maksim
Copy link
Member

@OddBloke
it is not utime, it is mtime -> m stands for modified

@beliaev-maksim
Copy link
Member

what is the difference between last updated and last modified?

@OddBloke
Copy link
Contributor Author

@OddBloke it is not utime, it is mtime -> m stands for modified

Right, mtime already exists: I was talking about the potential new name.

what is the difference between last updated and last modified?

https://stackoverflow.com/a/69496989 is the best explanation I've found.

@beliaev-maksim
Copy link
Member

@OddBloke
Got it, please submit a PR with an explicit name. I don't like current naming as well, but we should keep it for compatibility

OddBloke added a commit to OddBloke/artifactory that referenced this issue Sep 27, 2022
OddBloke added a commit to OddBloke/artifactory that referenced this issue Sep 27, 2022
@OddBloke
Copy link
Contributor Author

I've submitted #382: I could also submit a follow-up which deprecates mtime and ctime, if you would like?

OddBloke added a commit to OddBloke/artifactory that referenced this issue Sep 27, 2022
OddBloke added a commit to OddBloke/artifactory that referenced this issue Sep 29, 2022
@allburov
Copy link
Member

Will be released in 0.8.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Help Wanted We will be glad if somebody proposes a solution via PR
Development

No branches or pull requests

3 participants