On Wed, Jul 01, 2020 at 05:33:03PM +1000, Daniel Axtens wrote: > Rohit Sarkar <rohitsarkar5...@gmail.com> writes: > > > On Tue, Jun 30, 2020 at 11:59:52AM +1000, Daniel Axtens wrote: > >> Rohit Sarkar <rohitsarkar5...@gmail.com> writes: > >> > >> > Hey, > >> > On Wed, Jun 17, 2020 at 07:55:37AM +1000, Daniel Axtens wrote: > >> >> Rohit Sarkar <rohitsarkar5...@gmail.com> writes: > >> >> > >> >> > Hey, > >> >> > On Fri, Jun 12, 2020 at 12:04:54AM +0530, Rohit Sarkar wrote: > >> >> >> The parserelations command will be used to bulk import patch > >> >> >> relations into > >> >> >> Patchwork using a "patch groups" file. > >> >> >> The patch groups file is structured such that each line contains a > >> >> >> relation. > >> >> >> Eg patch groups file contents. > >> >> >> 1 3 4 > >> >> >> 2 > >> >> >> 5 9 10 > >> >> >> > >> >> >> In this case 2 relations will be ingested into Patchwork, (1,3,4) and > >> >> >> (5,9,10). Further group (5,9,10) also points to two upstream commit > >> >> >> references. > >> >> >> Note that before ingesting the relations the existing relations in > >> >> >> the > >> >> >> DB are removed. > >> >> >> > >> >> >> v1..v2 > >> >> >> - remove commit references from the patch groups file. > >> >> >> - Update documentation > >> >> >> - rename some variables to remove the overloading. > >> >> >> - use filter and update instead of get and save to reduce the db > >> >> >> calls. > >> >> >> (Visible performance enhancement) > >> >> >> > >> >> >> Leaving the copyright untouched till Ralf and Lukas comment on how to > >> >> >> proceed. > >> >> >> > >> >> >> Rohit Sarkar (1): > >> >> >> management: introduce parserelations command > >> >> >> > >> >> >> docs/deployment/management.rst | 26 +++++++ > >> >> >> .../management/commands/parserelations.py | 71 > >> >> >> +++++++++++++++++++ > >> >> >> patchwork/tests/test_management.py | 7 ++ > >> >> >> 3 files changed, 104 insertions(+) > >> >> >> create mode 100644 patchwork/management/commands/parserelations.py > >> >> >> > >> >> >> -- > >> >> >> 2.23.0.385.gbc12974a89 > >> >> >> > >> >> > > >> >> > Just wanted to follow up on this. Does this look good? > >> >> > >> >> I was thinking about this as I washed the dishes last night. > >> >> > >> >> Purging all relations in the database means that if any API-based user > >> >> of relations emerges before the big public servers adopt a release with > >> >> this, then they'll never be able to use it. > >> >> > >> >> What's the speed impact of doing two passes through the data, with pass > >> >> 1 collecting all the projects that this touches and pass 2 actually > >> >> installing the relations? > >> >> > >> >> Regards, > >> >> Daniel > >> > I wanted to reopen the discussion on this with the aim being to get > >> > everyone on the same page, and either go with this approach or decide on > >> > an alternate approach. > >> > > >> > The primary goal is to have a "parserelations" command that can parse a > >> > "patch-groups" file that contains a relation on each line. There are 2 > >> > approaches: > >> > > >> > a. Refresh the relations table. The parserelations command will remove > >> > the existing relations before inserting the newly parsed relations. This > >> > avoids any conflicts between existing and new relations. However other > >> > users of the API might be affected. > >> > > >> > b. Insert relations in a non destructive way. We need a way to handle > >> > conflicts between existing and incoming relations. So the solution is > >> > not really clear in this case. What should have precedence in the case > >> > of conflicts? The parserelations script or the existing relations. > >> > > >> > >> > >> Right, sorry for dropping this, work got a bit crazy. > >> > >> It occurs to me that I have an embedded assumption that may not be true > >> here. Are you trying to build a set of relations that includes > >> cross-project relations, or are you just trying to build a set of > >> relations entirely or mostly within a single patchwork project? > >> > >> I've been assuming that your relations are entirely or predominantly > >> intra-project, but I'm starting to suspect that maybe my understanding > >> is not right. > > The relations can be inter project too. PaStA is configured with a list > > of Patchwork projects for which it will pull all the corresponding > > patches and perform it's analyses. PaStA can relate patches across > > projects too. > > > >> If you are expecting a non-trivial quantity of cross-project relations, > >> then my suggested approach earlier (caller of the script specifies the > >> projects that patches may belong to) doesn't make sense. It probably > >> makes most sense to go with approach (a) in that case - it's really > >> difficult to figure out how to do (b) in a way that preserves the > >> meaning that any other API user, or the PaStA tool, is trying to create. > >> > >> If we do go down route (a), I want to make it really hard for users to > >> accidentally shoot themselves in the foot here and wipe out data. So > >> probably I would say that parserelations should only insert relations if > >> the table is empty when the script begins, and there should be a > >> separate command that with a suitably scary name that purges all > >> existing relations. > > > > I agree that we should make parserelations actions very explicit to > > prevent any loss of data. However, if we use parserelations to > > differentially import relations we will need to be able to insert > > relations with a non empty table. Let me explain the complete flow > > between PaStA and Patchwork so that things are a bit clear. > > We plan to run a script, maybe as a cron job at regular time intervals > > which does the following: > > 1. Exports patches from Patchwork into PaStA > > 2. Runs PaStA's analyses > > 3. Runs the parserelations script to parse the patch-groups file > > generated by PaStA and inserts the relations into Patchwork. In this > > step we refresh the db and insert all the relations afresh. A non > > destructive approach would involve thinking of a way to resolve > > conflicts between existing and new relations. > > OK. > > I am happy for you to stick with a destructive approach for the > managment command, so long as it's clear to the user that data is > potentially about to be destroyed. I'm happy for that to be: > > - two commands: a command that purges relations, and then a parse > command that checks if the relations table is empty and refuses to > run if it is not empty. > > - a single command that requires you to pass a command line option like > '--really-delete-old-relations' > > - a single command with a name that captures the idea that data will be > destroyed. I don't know what that would be, maybe 'replace...'. > > I'm not fussy which option you go with, I just don't want a command > named 'parse...' to delete data without the user being properly warned. Yeah, I agree. The name "parserelations" doesnt make it explicit enough. I am fine with replacerelations.
> (You will at some point need to consider how to do incremental updates > over the API for a publicly usable version, but that can definitely wait > and might be out of scope for your GSoC project.) Agreed. > Regards, > Daniel > > > > >> > >> Have I correctly understood things here? > >> > >> Regards, > >> Daniel > >> > > Thanks, > > Rohit Thanks Rohit _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
