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

Add/link api #13

Merged
merged 41 commits into from
Jan 16, 2017
Merged

Add/link api #13

merged 41 commits into from
Jan 16, 2017

Conversation

Chyroc-MD
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Jan 14, 2017

Coverage Status

Coverage increased (+10.6%) to 97.059% when pulling 020d9a8 on Add/link-api into 36fe1e3 on master.

Chyroc Chen added 2 commits January 14, 2017 14:29
return senPostRequest(url, JSON.stringify(body))
}

async updateLinkConfig (oldLinkName, config) {
Copy link
Member

Choose a reason for hiding this comment

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

why is this async and others are not?

let connectorName
let jdbcDriver
let connectionString
let fetchSize = linkConfig.hasOwnProperty('fetchSize') ? linkConfig['fetchSize'] : 1000
Copy link
Member

Choose a reason for hiding this comment

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

linkConfig('fetchSize') || 1000

Copy link
Member

Choose a reason for hiding this comment

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

same below

const port = linkConfig.hasOwnProperty('port') ? linkConfig['port'] : 3306
connectorName = 'generic-jdbc-connector'
jdbcDriver = 'com.mysql.jdbc.Driver'
connectionString = 'jdbc:mysql://' + linkConfig['host'] + ':' + port + '/' + linkConfig['databaseName']
Copy link
Member

Choose a reason for hiding this comment

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

you can use back tick to do string interpolation

throw new Error('linkType must be mysql')
}
return {
'links': [{
Copy link
Member

Choose a reason for hiding this comment

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

what is all this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

是说除了mysql还有什么吗,还是mysql那个的配置是什么含义。前者的话,除了mysql,还有oracle, generic,sftp,kite,kafka,ftp,hdfs。mysql属于generic。我再加个hdfs。

'username': 'root',
'password': '12343456',
}
await sqoopClient.updateLinkConfig(oldLinkName, config)
Copy link
Member

Choose a reason for hiding this comment

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

what is the return value of this update?

@coveralls
Copy link

coveralls commented Jan 14, 2017

Coverage Status

Coverage increased (+10.6%) to 97.059% when pulling 93e09bf on Add/link-api into 36fe1e3 on master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+10.6%) to 97.059% when pulling 93e09bf on Add/link-api into 36fe1e3 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+10.6%) to 97.059% when pulling 93e09bf on Add/link-api into 36fe1e3 on master.

const linkName = 'test_link_2'
await sqoopClient.updateLinkEnable(linkName)
const data = await sqoopClient.getLinkByLinkName(linkName)
expect(data['links'][0]['enabled'].toString()).to.equal('true')
Copy link
Member

Choose a reason for hiding this comment

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

you don't need toString

Copy link
Member

@jimexist jimexist left a comment

Choose a reason for hiding this comment

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

see comments

async deleteLinkAll () {
const data = await this.getLinkAll()
const deleteList = data['links'].map(link => this.deleteLink(link['name']))
return await deleteList
Copy link
Member

Choose a reason for hiding this comment

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

you should await Promise.all(a list of promise) not await a list of promise the latter does not work

@coveralls
Copy link

coveralls commented Jan 15, 2017

Coverage Status

Coverage increased (+8.2%) to 94.667% when pulling c0c0955 on Add/link-api into b777f75 on master.

@coveralls
Copy link

coveralls commented Jan 15, 2017

Coverage Status

Coverage increased (+8.3%) to 94.805% when pulling f86a7c2 on Add/link-api into b777f75 on master.

}
]
}
} else if (linkConfig['linkType'] === linkType.hdfs) {
Copy link
Member

Choose a reason for hiding this comment

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

tear up this into two smaller functions

@coveralls
Copy link

coveralls commented Jan 15, 2017

Coverage Status

Coverage increased (+8.9%) to 95.349% when pulling 2976264 on Add/link-api into b777f75 on master.

}

export function setCreateLinkRequestBody (linkConfig) {
if (linkConfig['linkType'] === linkType.mysql) {
Copy link
Member

Choose a reason for hiding this comment

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

use two separate functions for mysql and hdfs

Copy link
Member

@jimexist jimexist left a comment

Choose a reason for hiding this comment

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

see comment

@coveralls
Copy link

Coverage Status

Coverage increased (+9.0%) to 95.455% when pulling 1919690 on Add/link-api into b777f75 on master.

@coveralls
Copy link

coveralls commented Jan 16, 2017

Coverage Status

Coverage increased (+9.0%) to 95.455% when pulling 93f360f on Add/link-api into b777f75 on master.

@Chyroc-MD Chyroc-MD mentioned this pull request Jan 16, 2017
@@ -21,7 +30,7 @@ export class Hasoop {
}

formatUrl ([basicPath, queryObject = {}], ...otherPath) {
queryObject['user.name'] = this.userName
_.set(queryObject, ['user.name'], this.userName)
Copy link
Member

Choose a reason for hiding this comment

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

i guess ['user.name'] can be user.name

Copy link
Contributor Author

Choose a reason for hiding this comment

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

在lodash里面,它是以.分割的,如果直接写user.name的话,就会是user=,而不是user.name=<userName>

@Chyroc-MD Chyroc-MD merged commit 7cf2340 into master Jan 16, 2017
@jimexist jimexist deleted the Add/link-api branch January 16, 2017 03:59
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.

None yet

4 participants