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

202 restore multiples organizaciones #221

Merged
merged 11 commits into from
Nov 23, 2018

Conversation

lrromero
Copy link
Contributor

  • Las funcionalidades que estaban en restore_catalog_to_ckan() pasan a ser de restore_organization. restore_catalog se compone de varias llamadas a restore_organization
  • Agrega tests del comportamiento.
  • Agrega documentación.

@lrromero lrromero added this to In progress in Apertura - Sprint 24 via automation Nov 23, 2018
@coveralls
Copy link

coveralls commented Nov 23, 2018

Coverage Status

Coverage increased (+0.05%) to 82.326% when pulling 695e085 on 202-restore-multiples-organizaciones into e4a3fc2 on master.

for theme in themes
]

package['groups'] = package.get('groups', [])
Copy link
Contributor

Choose a reason for hiding this comment

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

Esto se logra mejor con package.setdefault('groups', [])

- **apikey**: La apikey de un usuario con los permisos que le permitan crear o actualizar el dataset.
- **dataset_list**: Los ids de los datasets a restaurar. Si no se pasa una lista, todos los datasests se restauran.
- **owner_org**: La organización a la cual pertencen los datasets.
- **download_strategy**: Una función (catálogo, distribución)->bool. Sobre las distribuciones que evalúa True,
Copy link
Contributor

Choose a reason for hiding this comment

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

Cambio brusco: no podríamos pasar las ids de distribuciones a subir directamente? Simplificaría la firma de la función

docs/MANUAL.md Outdated
parámetro. Toma los siguientes parámetros:
- **catalog**: El catálogo de origen que se restaura.
- **portal_url**: La URL del portal CKAN de destino.
- **apikey**: La apikey de un usuario con los permisos que le permitan crear o actualizar el dataset.
Copy link
Contributor

Choose a reason for hiding this comment

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

"el dataset" -> los datasets

"""
push_new_themes(catalog, portal_url, apikey)
if dataset_list is None:
dataset_list = [ds['identifier'] for ds in catalog.datasets]
Copy link
Contributor

Choose a reason for hiding this comment

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

Posible KeyError?

org_list = origin_portal.action.organization_list()
except CKANAPIError as e:
logger.exception(
'Ocurrió un buscando las organizaciones del portal {}: {}'
Copy link
Contributor

Choose a reason for hiding this comment

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

"ocurrió un buscando", falta la palabra "error", asumo

file_name = dist.get('fileName') or \
dist['downloadURL'].split('/')[-1]
os.rename(tmpfile.name, os.path.join(tmpdir, file_name))
tmpfile.name = os.path.join(tmpdir, file_name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Por qué todo el barullo de tempfile? Podés usar context managers para borrar el archivo en caso de un error

@poligarcia poligarcia merged commit 65465e1 into master Nov 23, 2018
@poligarcia poligarcia moved this from In progress to Done in Apertura - Sprint 24 Nov 26, 2018
@abenassi abenassi added the review label Dec 4, 2018
@lrromero lrromero deleted the 202-restore-multiples-organizaciones branch December 5, 2018 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants