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.

Sign up for GitHub

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

Jump to bottom

global: empty records fix #33

Merged
merged 4 commits into from
Jul 18, 2016
Merged

global: empty records fix #33

merged 4 commits into from
Jul 18, 2016

Conversation

krzysztof
Copy link
Member

@krzysztof krzysztof commented Jun 15, 2016 •
edited
Loading

  • Creates PID for empty records with recid.
  • Adds Oautclient migration.

Closes #42 #40
Closes #36

Signed-off-by: Krzysztof Nowak k.nowak@cern.ch

lnielsen
lnielsen reviewed Jun 15, 2016
View reviewed changes
invenio_migrator/records.py
# If 'record' is not present, just create the PID
if not dump.data.get('record'):
try:
PersistentIdentifier.get(pid_type='recid',
Copy link
Member

Choose a reason for hiding this comment

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

Should use https://github.com/inveniosoftware/invenio-pidstore/blob/master/invenio_pidstore/providers/recordid.py#L33

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I actually have to use the PI.create instead of provider since I am providing the recid as pid_value, which is forbidden in the provider: https://github.com/inveniosoftware/invenio-pidstore/blob/master/invenio_pidstore/providers/recordid.py#L53

@coveralls
Copy link

coveralls commented Jun 15, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.5%) to 60.294% when pulling 022a39f on krzysztof:migration_fixes into aae309f on inveniosoftware:master.

@krzysztof krzysztof changed the title global: empty records fix global: WIP empty records fix Jun 15, 2016
@coveralls
Copy link

coveralls commented Jun 16, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.5%) to 60.241% when pulling 98ce276 on krzysztof:migration_fixes into aae309f on inveniosoftware:master.

@jirikuncar jirikuncar changed the title global: WIP empty records fix WIP global: empty records fix Jun 17, 2016
@jirikuncar jirikuncar modified the milestones: v1.0.0, Sprint-Deposit Jun 17, 2016
@coveralls
Copy link

coveralls commented Jun 24, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-0.5%) to 60.0% when pulling fce8758 on krzysztof:migration_fixes into 8abb7fe on inveniosoftware:master.

@coveralls
Copy link

coveralls commented Jun 24, 2016 •
edited
Loading

Coverage Status

Coverage decreased (-2.8%) to 57.716% when pulling b1fb325 on krzysztof:migration_fixes into 8abb7fe on inveniosoftware:master.

@krzysztof
Copy link
Member Author

Please someone silence the coveralls guy:

  1. go to https://coveralls.io/github/inveniosoftware/invenio-migrator
  2. Untick "LEAVE COMMENTS?"
  3. Save

@krzysztof krzysztof changed the title WIP global: empty records fix global: empty records fix Jun 30, 2016
@krzysztof
Copy link
Member Author

LGTM

@egabancho
Copy link
Member

Coverage decreased (-2.8%) to 57.716%

@krzysztof do you think it would be possible to add some tests? 😉

@krzysztof
Copy link
Member Author

Coverage decreased (-1.06%) to 59.473%

@egabancho The leftovers is the legacy dumping stuff

@krzysztof
Copy link
Member Author

Also, this is pinning Sphinx==1.4.4...

@jirikuncar
Copy link
Member

@krzysztof usually it's caused by :undoc-members:. IMHO it's better to create __all__ in the module with functions/classes that should be documented.

@lnielsen lnielsen assigned krzysztof and unassigned krzysztof Jul 15, 2016
@krzysztof
Copy link
Member Author

Closes #42 #40

@krzysztof
Copy link
Member Author

krzysztof commented Jul 18, 2016 •
edited
Loading

LGTM, :shipit: ?

@krzysztof
Copy link
Member Author

krzysztof commented Jul 18, 2016 •
edited
Loading

@jirikuncar I think the issue with Sphinx comes from the fact that
the docs/api.rst is evaluating the proxy object here:
https://github.com/zenodo/invenio-migrator/blob/master/invenio_migrator/proxies.py#L32-L33

which is done outside of app context.

This can be disabled by including __all__ = () inside the proxies.py so that the current_migrator will not be documented, but I am not sure if that's the desired solution.

@lnielsen
Copy link
Member

Needs a rebase for fast-forward merge.

Krzysztof Nowak and others added 4 commits July 18, 2016 15:17
global: empty records fix
b641bbe 
* Creates PID for empty records with recid.

Signed-off-by: Krzysztof Nowak <k.nowak@cern.ch>
oauthclient: migration of remote accounts, tokens and identities
87b3f76 
* Adds migration for remote accounts, remote tokens and user identities.

* Code refactoring for loaders CLI and tasks.

Signed-off-by: Krzysztof Nowak <k.nowak@cern.ch>
@egabancho
tasks: import fix
9febc85 
* Moves package specific imports inside tasks to prevent import errors.

Signed-off-by: Esteban J. G. Gabancho <esteban.gabancho@gmail.com>
@jmartinm
users: load task fix
0bad28b 
* FIX Takes into account case where user password is empty. (closes #39)

Signed-off-by: Javier Martin Montull <javier.martin.montull@cern.ch>
@krzysztof
Copy link
Member Author

@lnielsen Should be ff-mergeable now.

@lnielsen lnielsen merged commit 0bad28b into inveniosoftware:master Jul 18, 2016
@krzysztof krzysztof mentioned this pull request Jul 18, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@krzysztof krzysztof

Labels
None yet
Projects
None yet
Milestone
Sprint-Deposit
Development

Successfully merging this pull request may close these issues.
6 participants
@krzysztof @coveralls @egabancho @jirikuncar @lnielsen @jmartinm