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

PR: Allow an adjustable time difference for modification detection (Editor) #22192

Open
wants to merge 3 commits into
base: 5.x
Choose a base branch
from

Conversation

and1bm
Copy link

@and1bm and1bm commented Jun 21, 2024

For CIFS (and perhaps other network file systems), where caching file attributes is used, expecting identical timestamps causes many false positives. This hacky patch allows for some deviation.

Description of Changes

  • Wrote at least one-line docstrings (for any new functions)
  • Added unit test(s) covering the changes (if testable)
  • Included a screenshot or animation (if affecting the UI, see Licecap)

Issue(s) Resolved

Fixes #21877 hopefully, we'll see next week when much more testing is planed in production.

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: and1bm

For CIFS (and perhaps other network file systems), where caching file attributes is used, expecting identical timestamps causes many false positives. This hacky patch allows for some deviation.
@pep8speaks
Copy link

pep8speaks commented Jun 21, 2024

Hello @and1bm! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-06-26 15:30:21 UTC

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Hey @and1bm, thanks for your contribution! I left a small comment for you, otherwise looks good to me.

spyder/plugins/editor/widgets/editor.py Outdated Show resolved Hide resolved
@ccordoba12 ccordoba12 added this to the v5.5.6 milestone Jun 25, 2024
@ccordoba12 ccordoba12 changed the title Allow an adjustable time difference for modification detection. PR: Allow an adjustable time difference for modification detection (Editor) Jun 25, 2024
@and1bm
Copy link
Author

and1bm commented Jun 26, 2024

Hi @ccordoba12, I reverted the changes in the translatable string.

@ccordoba12
Copy link
Member

Thanks! I think this is ready, so we just need your confirmation that it actually works in your systems to merge it.

@ccordoba12
Copy link
Member

@and1bm, any news about this patch? Did it work for you?

We're planning to release 5.5.6 soon and we'd like to include it in that release.

@and1bm
Copy link
Author

and1bm commented Jul 9, 2024

@ccordoba12 sorry, we are still investigating, it seems not to fix all (?) problems. We have actimeo=600 set for our cifs mount of the homes, perhaps it's related to that number. However, students are reporting slowly and I am waiting for more detailed reports to understand the issue and cifs access times better.

@ccordoba12
Copy link
Member

Ok, no problem. Let us know when you have more info.

@ccordoba12 ccordoba12 modified the milestones: v5.5.6, v6.0.1 Jul 9, 2024
@ccordoba12 ccordoba12 modified the milestones: v6.0.1, v6.0.2 Sep 6, 2024
@ccordoba12 ccordoba12 modified the milestones: v6.0.2, v6.x.x Sep 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants