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

Oracle migration example script fails in python3 #86785

Open
andf-crl opened this issue Aug 24, 2022 · 4 comments
Open

Oracle migration example script fails in python3 #86785

andf-crl opened this issue Aug 24, 2022 · 4 comments
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.

Comments

@andf-crl
Copy link

andf-crl commented Aug 24, 2022

Describe the problem

The fix-example.py script provided by engineering to DOCs team errors when run by python3, as python2's strip method has been deprecated ever since. This report came through to DOCS from the TSE org, having been reported by a client you've heard of who tried to run this script and received an error.

Please take a moment to update this script, sent to DOCS in 2019, for python3 as you can. Thank you!

To Reproduce

Follow migration process as described on doc, noting failure in step 4 when run with python3 instead of python2. Note also that doc specifies using python3. See DOC - 5461 for TSE report of error. See original v19.1 version of this page to see that original invocation specified python2. It seems somewhere along the way, DOCS updated the invocation step to python3 without testing that the script worked in python3. Today we have learned that it does not.

Expected behavior
Modern machines running modern python are able to migrate from Oracle to CRDB successfully.

Additional context
Original python script was born in 2019, released as part of this doc alongside CRDB v19.1. I spoke with @drewdeally who provided some context on the history of this script, and advised that we update it for python3. Thanks for the assist!

Jira issue: CRDB-18933

@andf-crl andf-crl added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Aug 24, 2022
@ianjevans
Copy link
Contributor

I think a simple fix would be to replace string.strip with str.strip() and don't bother with the map function.

for rec in reader:
        writer.writerow(rec.strip())

@andf-crl
Copy link
Author

Thanks @ianjevans I'll test out this change tomorrow.

@andf-crl
Copy link
Author

andf-crl commented Aug 25, 2022

@ianjevans I get the following similar, but different, suggestion from 2to3:

$ 2to3 fix-example.py
...
--- fix-example.py	(original)
+++ fix-example.py	(refactored)
...
-        writer.writerow(map(string.strip, rec))
+        writer.writerow(list(map(string.strip, rec)))

Running your suggestion through 2to3 results in a pass!

However, we'd need an Oracle instance, or a sample .lst file from one, to actually test this and verify that output is as expected with either of these suggested changes, or to see if there is a difference between them.

Copy link

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
10 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Projects
None yet
Development

No branches or pull requests

2 participants