Skip to content

[SERV-126] Generate binary audiowaveform data for audio resources #6

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Jun 17, 2021
Merged

[SERV-126] Generate binary audiowaveform data for audio resources #6

merged 10 commits into from
Jun 17, 2021

Conversation

@markmatney
Copy link

Notes:

  • "--bits", "8" was determined to be a good value experimentally; IMO it generates better-looking waveforms (not to mention that the live example we're familiar with also uses 8 bits)
  • I type-parameterized several instances of the Vert.x event bus methods since I thought it made them easier to reason about
  • the new integration test currently requires a real s3 bucket (and thus real credentials); to make it pass locally:
    1. follow these instructions from the README
    2. change audiowaveform.s3.bucket in test-config.properties to prod-audiowaveform-resources (that bucket is cleared out and ready -- the test doesn't clear it out automatically)

@markmatney markmatney requested a review from a team June 16, 2021 14:28
@markmatney markmatney self-assigned this Jun 16, 2021
ksclarke
ksclarke approved these changes Jun 16, 2021
View reviewed changes
ksclarke
ksclarke requested changes Jun 16, 2021
View reviewed changes
Copy link
Member

@ksclarke ksclarke left a comment

Choose a reason for hiding this comment

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

Actions failing: Command 'which audiowaveform' exited with code 1.

DRickard
DRickard reviewed Jun 16, 2021
View reviewed changes
Copy link
Member

@DRickard DRickard left a comment

Choose a reason for hiding this comment

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

I'm getting a build error, not certain if it's with the app or my environment... getting error
Could not resolve dependencies for project info.freelibrary:av-pairtree:jar:0.0.1-SNAPSHOT: The following artifacts could not be resolved: com.github.hazendaz:csveed:jar:0.7.1, info.freelibrary:vertx-pairtree:jar:0.0.0: Could not find artifact com.github.hazendaz:csveed:jar:0.7.1 in central (https://repo.maven.apache.org/maven2)
Digging through maven.apache.org, I find the csveed jars in https://repo.maven.apache.org/maven2/com/github/hazendaz/csveed/0.7.0/

@ksclarke
Copy link
Member

ksclarke commented Jun 16, 2021 •
edited
Loading

I'm getting a build error, not certain if it's with the app or my environment... getting error
Could not resolve dependencies for project info.freelibrary:av-pairtree:jar:0.0.1-SNAPSHOT: The following artifacts could not be resolved: com.github.hazendaz:csveed:jar:0.7.1, info.freelibrary:vertx-pairtree:jar:0.0.0: Could not find artifact com.github.hazendaz:csveed:jar:0.7.1 in central (https://repo.maven.apache.org/maven2)
Digging through maven.apache.org, I find the csveed jars in https://repo.maven.apache.org/maven2/com/github/hazendaz/csveed/0.7.0/

@DRickard did you run the two step build from the README? mvn validate && mvn verify -- because the jar is in the local lib dir it takes two steps: one to get it into the local repo and one for the full build.

It's modified from the upstream that's available in Maven Central.

@DRickard
Copy link
Member

@DRickard did you run the two step build from the README? mvn validate && mvn verify -- because the jar is in the local lib dir it takes two steps: one to get it into the local repo and one for the full build.

It's modified from the upstream that's available in Maven Central.

Read and follow the instructions? That's so crazy it just might work!

@DRickard DRickard closed this Jun 16, 2021
@DRickard DRickard reopened this Jun 16, 2021
@markmatney
Copy link
Author

The most recent commit addresses the failure of which audiowaveform; changes incoming to fix the missing AWS credentials.

@markmatney markmatney marked this pull request as draft June 16, 2021 21:21
@markmatney markmatney marked this pull request as ready for review June 16, 2021 22:24
@markmatney markmatney requested review from DRickard, cachemeoutside and ksclarke and removed request for cachemeoutside June 16, 2021 22:24
markmatney
markmatney commented Jun 16, 2021
View reviewed changes
Copy link
Author

@markmatney markmatney left a comment

Choose a reason for hiding this comment

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

@cachemeoutside I wanted you to review a couple points that I think I may have gotten wrong re: the URLs/identifiers for our audiowaveform data. Basically I forgot where we landed on these decisions.

ksclarke
ksclarke reviewed Jun 17, 2021
View reviewed changes
markmatney
markmatney commented Jun 17, 2021
View reviewed changes
@markmatney markmatney requested a review from ksclarke June 17, 2021 15:51
@markmatney markmatney removed the request for review from cachemeoutside June 17, 2021 20:02
@markmatney markmatney merged commit e558675 into main Jun 17, 2021
@markmatney markmatney deleted the SERV-126 branch June 17, 2021 20:09
@markmatney markmatney added the enhancement New feature or request label Jun 19, 2021
angelahuqing added a commit that referenced this pull request Mar 17, 2023
@angelahuqing
# This is a combination of 8 commits.
bb38017 
# This is the 1st commit message:

SERV-505 Added jobsQueue to watcherVerticle

# This is the commit message #2:

[SERV-505] Addition of JobsQueue Class and Test

# This is the commit message #3:

[SERV-505] Consistency update to CsvItemTest.java

# This is the commit message #4:

[SERV-505] Cleaned up JobsQueue code

# This is the commit message #5:

[SERV-505] Completion of JobsQueue functionality and added to WatcherVerticle

# This is the commit message #6:

[SERV-505] Added in messages

# This is the commit message #7:

[SERV-505] Fix checkstyle and delete unnecessary files

# This is the commit message #8:

[SERV-505] Fixed checkstyle violations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cachemeoutside cachemeoutside cachemeoutside left review comments

@ksclarke ksclarke ksclarke approved these changes

@DRickard DRickard DRickard approved these changes

Assignees

@markmatney markmatney

Labels

enhancement New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@markmatney @ksclarke @DRickard @cachemeoutside