I just had a quick look (quickest possible look) and a few things came to mind:
1. You did: "from pagure.lib import model" but never used it. Always did: "pagure.lib.model.something" :D
2. At two places (probably), i didn't like the space btw. session.add() and session.commit()
3. The second point of pep8 imports section. A little shuffling at the top might be required. :)

You could directly access the session via pagure.SESSION :)

Isn't this a little dangerous?

What's the second part of the line here?

rewrote the populate data script to choose either wipe the entire db if the user entered their own data or just wipe the sample data.

I just had a quick look (quickest possible look) and a few things came to mind:
1. You did: "from pagure.lib import model" but never used it. Always did: "pagure.lib.model.something" :D
2. At two places (probably), i didn't like the space btw. session.add() and session.commit()
3. The second point of pep8 imports section. A little shuffling at the top might be required. :)

Thanks for those hints i fixed it up. I'm using pycharm -community and i'm fairly certain the reformat file command is pep8 compliant

what about extracting this from the config file instead?

Why not raw_input(question + prompt).lower()?

i had it extracted before from "APP.config['DB_URL']" but for somereason i was worried that url would change to production on someones machine and wipe the production db. i guess its a unnecessary and will change it back

For this you could do plain SQL, DELETE * FROM %s; then session.execute(sql), that embedded in a loop should make the code easier to read.

What do you think?

Looking at the code, I'm not sure to understand the difference between empty_dev_db and delete_data?

this was a python recipe # Recipe 577058:

I can modify it so its cleaner

We could wonder about the necessity of wiping data in a script that's meant to add data :)

Maybe deleting stuff could/should(?) be left as an exercise to the reader? :D

It surely works as is, I was merely curious if there was a reason for now using raw_input() directly

empty_dev_db: wipes everything from db even if user's data they inserted
delete_data: only wipes the sample data from db

i consider delete_data safe since it only removes what this script added. while 'empty_dev_db' deletes everything and i consider it dangerous.

My thoughts for creating this function was that the dev or user could reset their local instance of the db to a fresh copy

check out line 33 the function 'empty_dev_db' would you consider hard coding the table names and iterating over the names instead?

No empty_dev_db is indeed nicer than what I was thinking :)

Updated script to make it simpler. Do you want me to squash my commits?

Maybe we can split this over multiple lines?

Maybe we could add an example with a specified configuration file?

We could let argparse know directly what the possible actions are, https://github.com/fedora-infra/packagedb-cli/blob/master/pkgdb2client/cli.py#L193 might help

Let's drop all the hooks, they can be set by the UI

We can drop this

We can drop this

Let's drop if it's taken care of

Maybe we could add someone to one of the group we created?

Add a group to one of the project created?

Isn't this taken care of by pagure.lib.model.create_tables() already?

Isn't this taken care of by pagure.lib.model.create_tables() already?

yes you are correct that function calls create_default_status() which does this ill remote this section from the code

yes it is ill remove this section

Not sure how to handle

• pull_request_flags
• issue_to_issue
• tags_projects

just figure out how to add data to

• user_project

Not sure how to handle
pull_request_flags

These are used for example by pagure-CI to give an idea how if tests are passing on a given PR.

So we could add a fake flag saying that Jenkins is happy (or not) about a PR

issue_to_issue

This is to link tickets together (depends on/requires)

tags_projects

These are used to allow easier filtering of the projects, we can likely ignore this for now

user_project

That's in the settings page of the project, it's basically adding a new contributor to a project

Seems like i got everything :) let me know otherwise.

Would you like me to squash my commits?

I'm kinda wondering if this function is really needed :)

It's printing a warning but the user can't do anything, should we ask for confirmation?

prevent unit-tests?

Let's drop that if we don't use it

This should be taken care of by argparse normally

Should we drop these?

adjusted the data and removed the src comments

pingou i pushed the commits before checking what you wrote :( so im going to have to guess what lines you were commenting on

This should be taken care of by argparse normally
i think you are refering to


if len(sys.argv) == 1:
parser.print_help()
sys.exit(1)


it wasn't handling that for me though :(

This should be taken care of by argparse normally
i think you are refering to

``

if len(sys.argv) == 1:

    parser.print_help()
    sys.exit(1)

``

That would be correct :)

pingou i think i addressed all your concerns

Looks good to me, I would like to give it a try locally before merging though :)

Sounds good :) let me know if its good and then ill squash my commits so the master has a clean history

Ok after testing I definitely like it with only one remark:

If you're not pingou, there are no projects for you.

If you are pingou, all the projects are yours.

So maybe cool to ask the user for his/her username and email and add it to one of the project and make the other project someone's else

Why the while here?

forces the user to enter something. unless you would prefer to have a default name and allow the user to change it

As you prefer :)

Im putting my shoes in the user and im lazy and i would probaby just spam enter. So perhaps adding a default name and asking the user if they would like to change it would be nice. if the input is empty just use default. Ill get that change up soon

just tested on a new folder and repo. tickets folder is now complaining. will need to add that check in.

tested again in a fresh clone and squashed my commits :)

Ok, works for me locally, let's merge :)