Opened 5 years ago

Closed 5 years ago

#20528 closed enhancement (fixed)

upgrade patchbot to 2.5.7

Reported by: chapoton Owned by:
Priority: major Milestone: sage-7.2
Component: packages: optional Keywords:
Cc: jdemeyer, vdelecroix, embray Merged in:
Authors: Frédéric Chapoton Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: e5a230d (Commits, GitHub, GitLab) Commit: e5a230d0395754fcbfd5afa2917198dbbea22b23
Dependencies: Stopgaps:

Status badges

Description (last modified by chapoton)

with a better treatment of startup modules

also correcting a detail about owner option

and no longer using os.getcwd()

Tarball: ​​​http://www-irma.u-strasbg.fr/~chapoton/patchbot-2.5.7.tar.bz2

Change History (35)

comment:1 Changed 5 years ago by chapoton

  • Branch set to public/20528
  • Commit set to 66cacb81e26e5a4ebfc1e4ad181c5dd9fcb6707c
  • Status changed from new to needs_review

New commits:

66cacb8update patchbot to 2.5.6

comment:2 Changed 5 years ago by chapoton

  • Cc jdemeyer vdelecroix embray added

comment:3 Changed 5 years ago by jdemeyer

  • Status changed from needs_review to needs_work
Found local metadata for patchbot-2.5.6
Invalid checksum for cached file /home/worker/sage-patchbot/upstream/patchbot-2.5.6.tar.bz2, deleting

comment:4 Changed 5 years ago by jdemeyer

ping?

comment:5 Changed 5 years ago by chapoton

I am not currently able to upload files to the place where I usually store my patchbot tars. So I cannot do anything for that ticket right now, sorry.

comment:6 Changed 5 years ago by git

  • Commit changed from 66cacb81e26e5a4ebfc1e4ad181c5dd9fcb6707c to 0b9787026bfd07d7fb7c9754804eb1a8b2282278

Branch pushed to git repo; I updated commit sha1. New commits:

92845e2Merge branch 'public/20528' into 7.2.rc1
0b97870fixing checksum for patchbot 2.5.6

comment:7 Changed 5 years ago by chapoton

  • Description modified (diff)
  • Status changed from needs_work to needs_review

ok, here is a corrected and working branch, with the tar.bz2 put on my website

comment:8 Changed 5 years ago by chapoton

  • Description modified (diff)

comment:9 Changed 5 years ago by jdemeyer

I will test this on sage4 and arando.

comment:10 Changed 5 years ago by chapoton

seems to be ok on sage4, but meeting problems in arando

comment:11 Changed 5 years ago by jdemeyer

The patchbot on arando is stuck on

[2016-05-12 09:40:11] The following tickets will be skipped: #15590 (until 1463039060.99)
[2016-05-12 09:40:11] Check base.
Traceback (most recent call last):
  File "/home/patchbot/sage-patchbot/local/bin/patchbot/patchbot.py", line 1432, in main
    if not patchbot.check_base():
  File "/home/patchbot/sage-patchbot/local/bin/patchbot/patchbot.py", line 689, in check_base
    cwd = os.getcwd()
OSError: [Errno 2] No such file or directory

comment:12 follow-up: Changed 5 years ago by chapoton

Hmm, this could be caused by a directory being removed, or by a filesystem problem. In which dir are you when launching the patchbot ? Have you removed this dir ?

I could wrap that with a try except clause. But I am using the variable cwd to get back to the original directory at the end of the function (by using os.chdir(cwd)). This is because I need to be in the sage_root directory when performing git operations.

EDIT: or maybe I could try to use os.getenv('PWD') instead ?

Last edited 5 years ago by chapoton (previous) (diff)

comment:13 in reply to: ↑ 12 Changed 5 years ago by jdemeyer

Replying to chapoton:

Hmm, this could be caused by a directory being removed, or by a filesystem problem. In which dir are you when launching the patchbot ?

/home/patchbot/sage-patchbot

Have you removed this dir ?

No.

But I am using the variable cwd to get back to the original directory

Is this really needed? I don't know the code exactly, but why can't you just chdir() to SAGE_ROOT and stay there?

If you really need to go back to the potentially-removed directory, you should use os.fchdir() instead of os.chdir() which will work even on deleted or moved directories.

comment:14 Changed 5 years ago by git

  • Commit changed from 0b9787026bfd07d7fb7c9754804eb1a8b2282278 to e5a230d0395754fcbfd5afa2917198dbbea22b23

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

e5a230dpatchbot 2.5.7, without using os.getcwd()

comment:15 Changed 5 years ago by chapoton

  • Description modified (diff)
  • Summary changed from upgrade patchbot to 2.5.6 to upgrade patchbot to 2.5.7

here is a new version not using os.getcwd, simply changing dir to sage root

comment:16 Changed 5 years ago by jdemeyer

I will test the new version on arando and sage4

Question (unrelated to this ticket): do you think that the patchbot should also test all optional packages with doctests? It might be a good idea to do that, to run all # optional tests depending on optional packages.

comment:17 Changed 5 years ago by jdemeyer

I saw this in the patchbot log while handling ticket #0. I don't know how serious it is.

Traceback (most recent call last):
  File "/home/patchbot/sage-patchbot/local/bin/patchbot/patchbot.py", line 1280, in report_ticket
    report['owner'] = self.owner
AttributeError: 'Patchbot' object has no attribute 'owner'

comment:18 Changed 5 years ago by chapoton

This has been solved in patchbot >= 2.5.6.

For which version of the bot do you see that ?

comment:19 follow-up: Changed 5 years ago by chapoton

It seems that the patchbot auto-downgrades itself to 2.5.5. Is this the normal behaviour ? I know that we auto-upgrade optional packages.. Anyway, this should not be a problem, once the patchbot is launched and running smoothly. But everytime you stop it, in this testing phase, you must be careful to re-install the latest version before re-launching.

comment:20 follow-up: Changed 5 years ago by chapoton

Concerning the optional doctests in general, I do not quite understand what is the status currently. Looking at the log

http://patchbot.sagemath.org/log/0/Ubuntu/14.04/i686/3.13.0-76-generic/arando/2016-05-13%2010:28:37?short

one sees the lines

Using --optional=4ti2,benzene,bliss,buckygen,cbc,ccache,coxeter3,csdp,database_cremona_ellcurve,database_gap,database_jones_numfield,database_mutation_class,database_odlyzko_zeta,database_pari,database_stein_watkins,database_symbolic_data,dot2tex,gambit,gap_packages,gdb,giac,giacpy,lrslib,mcqd,meataxe,modular_decomposition,mpir,patchbot,plantri,python2,qepcad,saclib,sage,tdlib,tides,topcom
Doctesting entire Sage library.

So it seems that indeed all optional things are tested. But maybe you did something special to achieve that ?

See also #20604 for fixing the optional doctest failing with coxeter3.

comment:21 in reply to: ↑ 20 Changed 5 years ago by jdemeyer

Replying to chapoton:

But maybe you did something special to achieve that ?

Yes, I manually installed all doctested optional packages. This was my question in 16: perhaps it would be nice to automatically do this.

comment:22 in reply to: ↑ 19 Changed 5 years ago by jdemeyer

Replying to chapoton:

I know that we auto-upgrade optional packages..

And we also auto-upgrade optional packages which have been "downgraded". I now installed 2.5.7 again on arando.

Last edited 5 years ago by jdemeyer (previous) (diff)

comment:23 Changed 5 years ago by jdemeyer

So far, things are looking good. I suggest to leave the bots running for a few more days and then set this to positive review.

comment:24 follow-ups: Changed 5 years ago by chapoton

Well, arando is still not happy with TOPCOM, for some reason.

see http://patchbot.sagemath.org/ticket/0/

By the way, I have started to try making the patchbot a correct python package. I would appreciate any help to do that. There is a branch here:

https://github.com/robertwb/sage-patchbot/branches

comment:25 in reply to: ↑ 24 Changed 5 years ago by jdemeyer

Replying to chapoton:

Well, arando is still not happy with TOPCOM, for some reason.

see http://patchbot.sagemath.org/ticket/0/

Really? But then why does it continue to run anyway, if there is a problem with ticket 0?

Last edited 5 years ago by jdemeyer (previous) (diff)

comment:26 in reply to: ↑ 24 Changed 5 years ago by jdemeyer

Replying to chapoton:

By the way, I have started to try making the patchbot a correct python package.

Did you look at the instructions on https://packaging.python.org/en/latest/distributing/

comment:27 follow-ups: Changed 5 years ago by chapoton

  • concerning arando, it should have stopped after failing on 0, unless you specified the option "skip_base=True". This being said, it does not meet on other tickets the failing doctest that we can see on ticket 0.
  • concerning the packaging, yes, I read that and did my best, but this is nevertheless my first try. And this change probably also require changing the inner gears of sage --patchbot"

comment:28 in reply to: ↑ 27 Changed 5 years ago by jdemeyer

Replying to chapoton:

  • concerning arando, it should have stopped after failing on 0, unless you specified the option "skip_base=True".

I don't think that I specified "skip_base=True", because I remember it breaking on other errors before (like the coxeter3 failure).

However, it seems that this test of ticket 0 was not the initial test, but some later test (maybe because a new release came out?). It seems that these later tests of ticket 0 do not cause the patchbot to abort.

comment:29 in reply to: ↑ 27 Changed 5 years ago by jdemeyer

Replying to chapoton:

  • concerning the packaging, yes, I read that and did my best, but this is nevertheless my first try.

I think it's best if you try to get started and then see if and where you get stuck.

comment:30 Changed 5 years ago by jdemeyer

Question: which tickets does the patchbot test? Only those which needs_review, or also positive_review or needs_info...?

comment:31 follow-up: Changed 5 years ago by chapoton

In principle all of them, but there is a priority given to "needs_review", I think. The problem is that we are currently seriously lacking running patchbots. Only poseidon is running, with a quite old but stable version of the bot. I do not know what happened to the other bots.

EDIT:

  default_bonus = {"needs_review": 1000,
    "positive_review": 500,"blocker": 100,"critical": 60,"major": 10,"unique": 40,"applies": 20,"behind": 1}
Last edited 5 years ago by chapoton (previous) (diff)

comment:32 in reply to: ↑ 31 Changed 5 years ago by jdemeyer

Replying to chapoton:

The problem is that we are currently seriously lacking running patchbots.

It doesn't look so bad to me.

Only poseidon is running

What about sage4? And I noticed that arando indeed went down, I'm restarting it now.

comment:33 Changed 5 years ago by chapoton

Maybe this could be considered as ready to go, no ?

I plan to work on "patchbot as a correct python pkg" after this is closed.

comment:34 Changed 5 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:35 Changed 5 years ago by vbraun

  • Branch changed from public/20528 to e5a230d0395754fcbfd5afa2917198dbbea22b23
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.