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

global: migrate zenodo github integration #8

Merged
merged 10 commits into from
Aug 1, 2016

Conversation

slint
Copy link
Member

@slint slint commented Jun 29, 2016

  • Migrates previous github implementation. (closes global: migrate zenodo github integration #8)
  • Adds status tracking for Releases.
  • Uses a Celery task for deposit publishing.
  • Uses GitHub's IDs to track Repositories and Releases.
  • Adds Bundles for the UI assets.
  • Adds webhook syncing.
  • Fixes various UI issues.

Signed-off-by: Alexander Ioannidis a.ioannidis@cern.ch

@slint slint changed the title WIP global: fixes and additions WIP global: migrate zenodo github integration Jun 29, 2016
@property
def doi(self):
if self.record:
return self.record.json.get('doi')
Copy link
Member

Choose a reason for hiding this comment

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

shall we use PID fetcher here?

@slint slint force-pushed the ui_migration branch 3 times, most recently from 75389aa to 86d532c Compare July 1, 2016 15:39
* Fixes several test cases by supplying missing dependencies such as
  Flask-CORS for Invenio-REST and by moving work-in-progress imports
  under corresponding test functions.

* Adds test database `test.db` to the git ignore list.

* Completes the `AUTHORS.rst` author list.

Signed-off-by: Tibor Simko <tibor.simko@cern.ch>
@slint slint force-pushed the ui_migration branch 6 times, most recently from 7cba5b7 to f0384e8 Compare July 6, 2016 19:33
release_id=release_id,
).first()
if existing_release:
msg = 'Release [{0}:{1}] has already been received with status {1}'
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be "Release [{0}:{1}] has already been received with status {2}"?

Copy link
Member

Choose a reason for hiding this comment

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

What about using names instead of numbers for string formatting?

'Release [{tag}:{release_id}] ...: {status}.'.format(tag=..., release_id=..., ...)

@slint slint force-pushed the ui_migration branch 15 times, most recently from 6c2c229 to 6aa42b9 Compare July 14, 2016 15:45
if sync_hooks:
self._sync_hook(gh_repo)

self._sync_hooks(active_repos.keys(), async=async_hooks)
Copy link
Member

Choose a reason for hiding this comment

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

Are we sure that we want alway to force hooks syncing?

Copy link
Member Author

@slint slint Jul 25, 2016

Choose a reason for hiding this comment

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

Currently the hook sync behaviour goes as:

  • Initial GitHub account connection OR Index page access after a full day without sync:
    • Triggers an async task that does the hook syncing (hook syncing happens passively).
  • User clicks on "Sync now..." button on index page:
    • Triggers a synchronous hook sync through the AJAX request which updates the page on the spot (hook syncing happens on-demand).

Copy link
Member

Choose a reason for hiding this comment

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

My question was more about keep the sync_hooks parameter so you can sync everything but hooks.

@coveralls
Copy link

coveralls commented Jul 25, 2016

Coverage Status

Changes Unknown when pulling 4e30a57 on slint:ui_migration into * on inveniosoftware:master*.


def _(text):
"""Identity function for `gettext`."""
# FIXME: Replace with `speaklater.make_lazy_gettext`
Copy link
Member

Choose a reason for hiding this comment

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

or better flask_babelex.lazy_gettext

@jirikuncar jirikuncar assigned slint and unassigned lnielsen Jul 26, 2016
* Fixes issue with current_webhooks only being available on API
  application by forcing GITHUB_WEBHOOK_RECEIVER_URL to be set.

Signed-off-by: Lars Holm Nielsen <lars.holm.nielsen@cern.ch>
Signed-off-by: Lars Holm Nielsen <lars.holm.nielsen@cern.ch>
@@ -331,31 +321,29 @@ def repository(self):
"""Return repository metadata."""
return self.payload['repository']

@property
def repository_model(self):
@cached_property
Copy link
Member

Choose a reason for hiding this comment

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

This might be dangerous because you can get an object bounded to a different session or transaction ...

slint and others added 3 commits July 29, 2016 18:02
* Handles OAuth signals for account setup and hook syncing.

* Adds a default OAuth remote application configuration.

Signed-off-by: Alexander Ioannidis <a.ioannidis@cern.ch>
Signed-off-by: Lars Holm Nielsen <lars.holm.nielsen@cern.ch>
* Adds proper documentation.

* Refactors Repository model's get and create methods.

* Adds custom exceptions.

* Remove unused static files.

Signed-off-by: Alexander Ioannidis <a.ioannidis@cern.ch>
@slint slint force-pushed the ui_migration branch 2 times, most recently from 7bf2984 to 66a8f84 Compare July 29, 2016 17:05
* Reduces remote GitHub API calls.

* Migrates methods from Release model to GitHubRelease.

* Uses PID Fetcher for Release records.

* Integrates GitHubRelease in views.

* Adds exception handling and proper HTTP status code responses.

Signed-off-by: Alexander Ioannidis <a.ioannidis@cern.ch>
@lnielsen lnielsen merged commit 1d36e05 into inveniosoftware:master Aug 1, 2016
@slint slint deleted the ui_migration branch August 17, 2016 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants