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

cli: fixes replace method #2046

Merged
merged 1 commit into from
Dec 14, 2017

Conversation

ioannistsanaktsidis
Copy link
Contributor

Signed-off-by: Ioannis Tsanaktsidis [email protected]

@ioannistsanaktsidis
Copy link
Contributor Author

@tiborsimko please check and merge if everything is ok. I think the problem is fixed now .

@tiborsimko
Copy link
Member

@ioannistsanaktsidis I think this would not update the files, i.e. the scenario described in the issue would not be covered by this PR. See also my comment #2039 (comment)

@ioannistsanaktsidis
Copy link
Contributor Author

@tiborsimko I updated with @pamfilos the PR. You can have a look now. I think it covers all our cases.

@ioannistsanaktsidis ioannistsanaktsidis force-pushed the fixes-replace branch 4 times, most recently from e7f93f7 to 3d950f9 Compare December 13, 2017 14:55
@tiborsimko
Copy link
Member

Does not seem to work fully for me, here's one use case:

# clean instance:
./scripts/clean-instance.sh
./scripts/populate-instance.sh --skip-records
# 1) upload a record without files:
cernopendata fixtures records -f ./cernopendata/modules/fixtures/data/records/cms-tools-vm-image.json --mode insert --skip-files
firefox http://0.0.0.0:5000/record/249
# ... file names not visible, which is good
curl http://0.0.0.0:5000/record/249/files/cms-user-data.txt
# ... gives "Page not found", which is good
# 2) reupload the same record with files:
cernopendata fixtures records -f ./cernopendata/modules/fixtures/data/records/cms-tools-vm-image.json --mode replace
firefox http://0.0.0.0:5000/record/249
# ... file names are now visible, which is good
curl http://0.0.0.0:5000/record/249/files/cms-user-data.txt
# ... but still returns "Page not found", which is bad
# 3) reupload the same record once more:
cernopendata fixtures records -f ./cernopendata/modules/fixtures/data/records/cms-tools-vm-image.json --mode replace
# ... gives exception sqlalchemy.exc.InvalidRequestError: This Session's transaction has been rolled back due to a previous exception during flush. To begin a new transaction with this Session, first issue Session.rollback(). Original exception was: (psycopg2.IntegrityError) duplicate key value violates unique constraint "uq_files_files_uri"
DETAIL:  Key (uri)=(root://eospublic.cern.ch//eos/opendata/cms/environment/2010/CMS-OpenData-1.0.0-rc7.ova) already exists.
 [SQL: 'INSERT INTO files_files (created, updated, id, uri, storage_class, size, checksum, readable, writable, last_check_at, last_check) VALUES (%(created)s, %(updated)s, %(id)s, %(uri)s, %(storage_class)s, %(size)s, %(checksum)s, %(readable)s, %(writable)s, %(last_check_at)s, %(last_check)s)'] [parameters: {'updated': datetime.datetime(2017, 12, 14, 1, 56, 51, 400905), 'created': datetime.datetime(2017, 12, 14, 1, 56, 51, 400898), 'checksum': u'sha1:3a5d1ca5041a83a0db1c835b6e7af18d46e2ceb0', 'last_check_at': None, 'readable': True, 'uri': u'root://eospublic.cern.ch//eos/opendata/cms/environment/2010/CMS-OpenData-1.0.0-rc7.ova', 'last_check': True, 'writable': False, 'storage_class': 'S', 'id': UUID('c4b1b2a4-3fdd-4b6e-9e18-4480118c2fbc'), 'size': 9159569}]

@ioannistsanaktsidis ioannistsanaktsidis force-pushed the fixes-replace branch 2 times, most recently from 88afc4a to a9ee7ad Compare December 14, 2017 12:32
 * Closes cernopendata#2039.

Signed-off-by: Ioannis Tsanaktsidis <[email protected]>
Co-authored-by: Pamfilos Fokianos <[email protected]>
Copy link
Member

@tiborsimko tiborsimko left a comment

Choose a reason for hiding this comment

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

Works well for the common use cases. I'll create a someday ticket for one corner case observation.

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.

2 participants