Skip to content

Conversation

@ErinWeisbart
Copy link
Member

@ErinWeisbart ErinWeisbart commented Dec 16, 2022

Cleanup of many open issues.

@ErinWeisbart ErinWeisbart linked an issue Dec 16, 2022 that may be closed by this pull request
@ErinWeisbart
Copy link
Member Author

@bethac07 This is ready to review/merge, other than merge conflicts, which send me into a panic trying to resolve. I'm sorry I've created another behemoth of a PR. It started in my head as just a bunch of little cleanup so it made sense to clump it all together, but seems to have grown...


### Creating File Lists

Use any text editing software to create a `.txt` file where each line of the file is a path to a single image that you want to process.
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should mention here that it needs to be abspaths, not relpaths.

run.py Outdated
print("Service created")


def get_queue_url(sqs):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why change this function instead of just run it twice, once for the dead letter queue and then once for the regular queue?

Copy link
Member Author

Choose a reason for hiding this comment

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

We used to pass SQS_DEAD_LETTER_QUEUE as an ARN not a name so it had to handle them each differently. Then I needed to change it from ARN to a name as the easiest way to create it if it doesn't exist and then I missed cleaning this part up after myself :)

@bethac07
Copy link
Collaborator

I did the best I could with the merge commit, but it had indeed gotten messy. It should be tested (preferably after addressing upstream comments) before we pull

else:
printandlog('== ERR \n'+err,logger)
mvtries+=1
printandlog('Move attempt #'+str(mvtries+1),logger)
Copy link
Collaborator

Choose a reason for hiding this comment

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

f string?

printandlog('== ERR \n'+err,logger)
mvtries+=1
printandlog('Move attempt #'+str(mvtries+1),logger)
cmd = 'aws s3 mv ' + localOut + ' s3://' + DESTINATION_BUCKET + '/' + remoteOut + ' --recursive --exclude=cp.is.done'
Copy link
Collaborator

Choose a reason for hiding this comment

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

f string?

cmd = 'aws s3 mv ' + localOut + ' s3://' + DESTINATION_BUCKET + '/' + remoteOut + ' --recursive --exclude=cp.is.done'
if UPLOAD_FLAGS:
cmd += ' ' + UPLOAD_FLAGS
printandlog('Uploading with command ' + cmd, logger)
Copy link
Collaborator

Choose a reason for hiding this comment

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

f string?

@bethac07
Copy link
Collaborator

Does this also resolve #114 and #131?

@ErinWeisbart ErinWeisbart marked this pull request as ready for review January 23, 2023 22:56
@ErinWeisbart ErinWeisbart merged commit e226968 into master Jan 24, 2023
@ErinWeisbart ErinWeisbart deleted the erin_cleanup branch January 24, 2023 01:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants