Fix Estimator role expansion - #68

Merged
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role
Feb 2, 2018
Merged

Fix Estimator role expansion#68
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role

Conversation

@iquintero

Copy link
Copy Markdown
Contributor

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.

This fixes#43

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
@iquintero
iquintero requested a review from owen-tFebruary 1, 2018 18:57

@mvsuspmvsusp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iquintero, can we write a unit test for that?

@iquintero

Copy link
Copy Markdown
ContributorAuthor

It doesn't really make sense to have a unit test for this, as I would have to mock the boto call to IAM and at that point there is nothing else left to test. Also the ARN is not being manually crafted anymore so we should be able to trust that boto is doing the right thing here.

@mvsusp

Copy link
Copy Markdown
Contributor

Agreed.

@iquintero
iquintero merged commit b45d79c into aws:masterFeb 2, 2018
jalabort added a commit to hudl/sagemaker-python-sdk that referenced this pull request Mar 1, 2018
* Add data_type to hyperparameters (aws#54)
When we describe a training job the data type of the hyper parameters is
lost because we use a dict[str, str]. This adds a new field to
Hyperparameter so that we can convert the datatypes at runtime.
instead of validating with isinstance(), we cast the hp value to the type it
is meant to be. This enforces a "strongly typed" value. When we
deserialize from the API string responses it becomes easier to deal with
too.
* Add wrapper for LDA. (aws#56)
Update CHANGELOG and bump the version number.
* Add support for async fit() (aws#59)
when calling fit(wait=False) it will return immediately. The training
job will carry on even if the process exits. by using attach() the
estimator can be retrieved by providing the training job name.
_prepare_init_params_from_job_description() is now a classmethod instead
of being a static method. Each class is responsible to implement their
specific logic to convert a training job description into arguments that
can be passed to its own __init__()
* Fix Estimator role expansion (aws#68)
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
* Add FM and LDA to the documentation. (aws#66)
* Fix description of an argument of sagemaker.session.train (aws#69)
* Fix description of an argument of sagemaker.session.train
'input_config' should be an array which has channel objects.
* Add a link to the botocore docs
* Use 'list' instead of 'array' in the description
* Add ntm algorithm with doc, unit tests, integ tests (aws#73)
* JSON serializer: predictor.predict accepts dictionaries (aws#62)
Add support for serializing python dictionaries to json
Add prediction with dictionary in tf iris integ test
* Fixing timeouts for PCA async integration test. (aws#78)
Execute tf_cifar test without logs to eliminate delay to detect that job has finished.
* Fixes in LinearLearner and unit tests addition. (aws#77)
* Print out billable seconds after training completes (aws#30)
* Added: print out billable seconds after training completes
* Fixed: test_session.py to pass unit tests
* Fixed: removed offending tzlocal()
* Use sagemaker_timestamp when creating endpoint names in integration tests. (aws#81)
* Support TensorFlow-1.5.0 and MXNet-1.0.0 (aws#82)
* Update .gitignore to ignore pytest_cache.
* Support TensorFlow-1.5.0 and MXNet-1.0.0
* Update and refactor tests. Add tests for fw_utils.
* Fix typo.
* Update changelog for 1.1.0 (aws#85)
@iquintero
iquintero deleted the service_role branch April 2, 2018 00:05
apacker pushed a commit to apacker/sagemaker-python-sdk that referenced this pull request Nov 15, 2018
Updated: XGBoost direct marketing notebook markdown at the end
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
Sign up for freeto 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.

creating estimator with role name fails if role is default sagemaker service role

2 participants

@iquintero@mvsusp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Fix Estimator role expansion - #68

Merged
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role
Feb 2, 2018
Merged

Fix Estimator role expansion#68
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role

Conversation

@iquintero

Copy link
Copy Markdown
Contributor

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.

This fixes#43

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
@iquintero
iquintero requested a review from owen-tFebruary 1, 2018 18:57

@mvsuspmvsusp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iquintero, can we write a unit test for that?

@iquintero

Copy link
Copy Markdown
ContributorAuthor

It doesn't really make sense to have a unit test for this, as I would have to mock the boto call to IAM and at that point there is nothing else left to test. Also the ARN is not being manually crafted anymore so we should be able to trust that boto is doing the right thing here.

@mvsusp

Copy link
Copy Markdown
Contributor

Agreed.

@iquintero
iquintero merged commit b45d79c into aws:masterFeb 2, 2018
jalabort added a commit to hudl/sagemaker-python-sdk that referenced this pull request Mar 1, 2018
* Add data_type to hyperparameters (aws#54)
When we describe a training job the data type of the hyper parameters is
lost because we use a dict[str, str]. This adds a new field to
Hyperparameter so that we can convert the datatypes at runtime.
instead of validating with isinstance(), we cast the hp value to the type it
is meant to be. This enforces a "strongly typed" value. When we
deserialize from the API string responses it becomes easier to deal with
too.
* Add wrapper for LDA. (aws#56)
Update CHANGELOG and bump the version number.
* Add support for async fit() (aws#59)
when calling fit(wait=False) it will return immediately. The training
job will carry on even if the process exits. by using attach() the
estimator can be retrieved by providing the training job name.
_prepare_init_params_from_job_description() is now a classmethod instead
of being a static method. Each class is responsible to implement their
specific logic to convert a training job description into arguments that
can be passed to its own __init__()
* Fix Estimator role expansion (aws#68)
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
* Add FM and LDA to the documentation. (aws#66)
* Fix description of an argument of sagemaker.session.train (aws#69)
* Fix description of an argument of sagemaker.session.train
'input_config' should be an array which has channel objects.
* Add a link to the botocore docs
* Use 'list' instead of 'array' in the description
* Add ntm algorithm with doc, unit tests, integ tests (aws#73)
* JSON serializer: predictor.predict accepts dictionaries (aws#62)
Add support for serializing python dictionaries to json
Add prediction with dictionary in tf iris integ test
* Fixing timeouts for PCA async integration test. (aws#78)
Execute tf_cifar test without logs to eliminate delay to detect that job has finished.
* Fixes in LinearLearner and unit tests addition. (aws#77)
* Print out billable seconds after training completes (aws#30)
* Added: print out billable seconds after training completes
* Fixed: test_session.py to pass unit tests
* Fixed: removed offending tzlocal()
* Use sagemaker_timestamp when creating endpoint names in integration tests. (aws#81)
* Support TensorFlow-1.5.0 and MXNet-1.0.0 (aws#82)
* Update .gitignore to ignore pytest_cache.
* Support TensorFlow-1.5.0 and MXNet-1.0.0
* Update and refactor tests. Add tests for fw_utils.
* Fix typo.
* Update changelog for 1.1.0 (aws#85)
@iquintero
iquintero deleted the service_role branch April 2, 2018 00:05
apacker pushed a commit to apacker/sagemaker-python-sdk that referenced this pull request Nov 15, 2018
Updated: XGBoost direct marketing notebook markdown at the end
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
Sign up for freeto 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.

creating estimator with role name fails if role is default sagemaker service role

2 participants

@iquintero@mvsusp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix Estimator role expansion - #68

Merged
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role
Feb 2, 2018
Merged

Fix Estimator role expansion#68
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role

Conversation

@iquintero

Copy link
Copy Markdown
Contributor

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.

This fixes#43

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
@iquintero
iquintero requested a review from owen-tFebruary 1, 2018 18:57

@mvsuspmvsusp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iquintero, can we write a unit test for that?

@iquintero

Copy link
Copy Markdown
ContributorAuthor

It doesn't really make sense to have a unit test for this, as I would have to mock the boto call to IAM and at that point there is nothing else left to test. Also the ARN is not being manually crafted anymore so we should be able to trust that boto is doing the right thing here.

@mvsusp

Copy link
Copy Markdown
Contributor

Agreed.

@iquintero
iquintero merged commit b45d79c into aws:masterFeb 2, 2018
jalabort added a commit to hudl/sagemaker-python-sdk that referenced this pull request Mar 1, 2018
* Add data_type to hyperparameters (aws#54)
When we describe a training job the data type of the hyper parameters is
lost because we use a dict[str, str]. This adds a new field to
Hyperparameter so that we can convert the datatypes at runtime.
instead of validating with isinstance(), we cast the hp value to the type it
is meant to be. This enforces a "strongly typed" value. When we
deserialize from the API string responses it becomes easier to deal with
too.
* Add wrapper for LDA. (aws#56)
Update CHANGELOG and bump the version number.
* Add support for async fit() (aws#59)
when calling fit(wait=False) it will return immediately. The training
job will carry on even if the process exits. by using attach() the
estimator can be retrieved by providing the training job name.
_prepare_init_params_from_job_description() is now a classmethod instead
of being a static method. Each class is responsible to implement their
specific logic to convert a training job description into arguments that
can be passed to its own __init__()
* Fix Estimator role expansion (aws#68)
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
* Add FM and LDA to the documentation. (aws#66)
* Fix description of an argument of sagemaker.session.train (aws#69)
* Fix description of an argument of sagemaker.session.train
'input_config' should be an array which has channel objects.
* Add a link to the botocore docs
* Use 'list' instead of 'array' in the description
* Add ntm algorithm with doc, unit tests, integ tests (aws#73)
* JSON serializer: predictor.predict accepts dictionaries (aws#62)
Add support for serializing python dictionaries to json
Add prediction with dictionary in tf iris integ test
* Fixing timeouts for PCA async integration test. (aws#78)
Execute tf_cifar test without logs to eliminate delay to detect that job has finished.
* Fixes in LinearLearner and unit tests addition. (aws#77)
* Print out billable seconds after training completes (aws#30)
* Added: print out billable seconds after training completes
* Fixed: test_session.py to pass unit tests
* Fixed: removed offending tzlocal()
* Use sagemaker_timestamp when creating endpoint names in integration tests. (aws#81)
* Support TensorFlow-1.5.0 and MXNet-1.0.0 (aws#82)
* Update .gitignore to ignore pytest_cache.
* Support TensorFlow-1.5.0 and MXNet-1.0.0
* Update and refactor tests. Add tests for fw_utils.
* Fix typo.
* Update changelog for 1.1.0 (aws#85)
@iquintero
iquintero deleted the service_role branch April 2, 2018 00:05
apacker pushed a commit to apacker/sagemaker-python-sdk that referenced this pull request Nov 15, 2018
Updated: XGBoost direct marketing notebook markdown at the end
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
Sign up for freeto 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.

creating estimator with role name fails if role is default sagemaker service role

2 participants

@iquintero@mvsusp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix Estimator role expansion - #68

Merged
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role
Feb 2, 2018
Merged

Fix Estimator role expansion#68
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role

Conversation

@iquintero

Copy link
Copy Markdown
Contributor

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.

This fixes#43

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
@iquintero
iquintero requested a review from owen-tFebruary 1, 2018 18:57

@mvsuspmvsusp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iquintero, can we write a unit test for that?

@iquintero

Copy link
Copy Markdown
ContributorAuthor

It doesn't really make sense to have a unit test for this, as I would have to mock the boto call to IAM and at that point there is nothing else left to test. Also the ARN is not being manually crafted anymore so we should be able to trust that boto is doing the right thing here.

@mvsusp

Copy link
Copy Markdown
Contributor

Agreed.

@iquintero
iquintero merged commit b45d79c into aws:masterFeb 2, 2018
jalabort added a commit to hudl/sagemaker-python-sdk that referenced this pull request Mar 1, 2018
* Add data_type to hyperparameters (aws#54)
When we describe a training job the data type of the hyper parameters is
lost because we use a dict[str, str]. This adds a new field to
Hyperparameter so that we can convert the datatypes at runtime.
instead of validating with isinstance(), we cast the hp value to the type it
is meant to be. This enforces a "strongly typed" value. When we
deserialize from the API string responses it becomes easier to deal with
too.
* Add wrapper for LDA. (aws#56)
Update CHANGELOG and bump the version number.
* Add support for async fit() (aws#59)
when calling fit(wait=False) it will return immediately. The training
job will carry on even if the process exits. by using attach() the
estimator can be retrieved by providing the training job name.
_prepare_init_params_from_job_description() is now a classmethod instead
of being a static method. Each class is responsible to implement their
specific logic to convert a training job description into arguments that
can be passed to its own __init__()
* Fix Estimator role expansion (aws#68)
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
* Add FM and LDA to the documentation. (aws#66)
* Fix description of an argument of sagemaker.session.train (aws#69)
* Fix description of an argument of sagemaker.session.train
'input_config' should be an array which has channel objects.
* Add a link to the botocore docs
* Use 'list' instead of 'array' in the description
* Add ntm algorithm with doc, unit tests, integ tests (aws#73)
* JSON serializer: predictor.predict accepts dictionaries (aws#62)
Add support for serializing python dictionaries to json
Add prediction with dictionary in tf iris integ test
* Fixing timeouts for PCA async integration test. (aws#78)
Execute tf_cifar test without logs to eliminate delay to detect that job has finished.
* Fixes in LinearLearner and unit tests addition. (aws#77)
* Print out billable seconds after training completes (aws#30)
* Added: print out billable seconds after training completes
* Fixed: test_session.py to pass unit tests
* Fixed: removed offending tzlocal()
* Use sagemaker_timestamp when creating endpoint names in integration tests. (aws#81)
* Support TensorFlow-1.5.0 and MXNet-1.0.0 (aws#82)
* Update .gitignore to ignore pytest_cache.
* Support TensorFlow-1.5.0 and MXNet-1.0.0
* Update and refactor tests. Add tests for fw_utils.
* Fix typo.
* Update changelog for 1.1.0 (aws#85)
@iquintero
iquintero deleted the service_role branch April 2, 2018 00:05
apacker pushed a commit to apacker/sagemaker-python-sdk that referenced this pull request Nov 15, 2018
Updated: XGBoost direct marketing notebook markdown at the end
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
Sign up for freeto 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.

creating estimator with role name fails if role is default sagemaker service role

2 participants

@iquintero@mvsusp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Fix Estimator role expansion - #68

Merged
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role
Feb 2, 2018
Merged

Fix Estimator role expansion#68
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role

Conversation

@iquintero

Copy link
Copy Markdown
Contributor

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.

This fixes#43

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
@iquintero
iquintero requested a review from owen-tFebruary 1, 2018 18:57

@mvsuspmvsusp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iquintero, can we write a unit test for that?

@iquintero

Copy link
Copy Markdown
ContributorAuthor

It doesn't really make sense to have a unit test for this, as I would have to mock the boto call to IAM and at that point there is nothing else left to test. Also the ARN is not being manually crafted anymore so we should be able to trust that boto is doing the right thing here.

@mvsusp

Copy link
Copy Markdown
Contributor

Agreed.

@iquintero
iquintero merged commit b45d79c into aws:masterFeb 2, 2018
jalabort added a commit to hudl/sagemaker-python-sdk that referenced this pull request Mar 1, 2018
* Add data_type to hyperparameters (aws#54)
When we describe a training job the data type of the hyper parameters is
lost because we use a dict[str, str]. This adds a new field to
Hyperparameter so that we can convert the datatypes at runtime.
instead of validating with isinstance(), we cast the hp value to the type it
is meant to be. This enforces a "strongly typed" value. When we
deserialize from the API string responses it becomes easier to deal with
too.
* Add wrapper for LDA. (aws#56)
Update CHANGELOG and bump the version number.
* Add support for async fit() (aws#59)
when calling fit(wait=False) it will return immediately. The training
job will carry on even if the process exits. by using attach() the
estimator can be retrieved by providing the training job name.
_prepare_init_params_from_job_description() is now a classmethod instead
of being a static method. Each class is responsible to implement their
specific logic to convert a training job description into arguments that
can be passed to its own __init__()
* Fix Estimator role expansion (aws#68)
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
* Add FM and LDA to the documentation. (aws#66)
* Fix description of an argument of sagemaker.session.train (aws#69)
* Fix description of an argument of sagemaker.session.train
'input_config' should be an array which has channel objects.
* Add a link to the botocore docs
* Use 'list' instead of 'array' in the description
* Add ntm algorithm with doc, unit tests, integ tests (aws#73)
* JSON serializer: predictor.predict accepts dictionaries (aws#62)
Add support for serializing python dictionaries to json
Add prediction with dictionary in tf iris integ test
* Fixing timeouts for PCA async integration test. (aws#78)
Execute tf_cifar test without logs to eliminate delay to detect that job has finished.
* Fixes in LinearLearner and unit tests addition. (aws#77)
* Print out billable seconds after training completes (aws#30)
* Added: print out billable seconds after training completes
* Fixed: test_session.py to pass unit tests
* Fixed: removed offending tzlocal()
* Use sagemaker_timestamp when creating endpoint names in integration tests. (aws#81)
* Support TensorFlow-1.5.0 and MXNet-1.0.0 (aws#82)
* Update .gitignore to ignore pytest_cache.
* Support TensorFlow-1.5.0 and MXNet-1.0.0
* Update and refactor tests. Add tests for fw_utils.
* Fix typo.
* Update changelog for 1.1.0 (aws#85)
@iquintero
iquintero deleted the service_role branch April 2, 2018 00:05
apacker pushed a commit to apacker/sagemaker-python-sdk that referenced this pull request Nov 15, 2018
Updated: XGBoost direct marketing notebook markdown at the end
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
Sign up for freeto 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.

creating estimator with role name fails if role is default sagemaker service role

2 participants

@iquintero@mvsusp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix Estimator role expansion - #68

Merged
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role
Feb 2, 2018
Merged

Fix Estimator role expansion#68
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role

Conversation

@iquintero

Copy link
Copy Markdown
Contributor

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.

This fixes#43

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
@iquintero
iquintero requested a review from owen-tFebruary 1, 2018 18:57

@mvsuspmvsusp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iquintero, can we write a unit test for that?

@iquintero

Copy link
Copy Markdown
ContributorAuthor

It doesn't really make sense to have a unit test for this, as I would have to mock the boto call to IAM and at that point there is nothing else left to test. Also the ARN is not being manually crafted anymore so we should be able to trust that boto is doing the right thing here.

@mvsusp

Copy link
Copy Markdown
Contributor

Agreed.

@iquintero
iquintero merged commit b45d79c into aws:masterFeb 2, 2018
jalabort added a commit to hudl/sagemaker-python-sdk that referenced this pull request Mar 1, 2018
* Add data_type to hyperparameters (aws#54)
When we describe a training job the data type of the hyper parameters is
lost because we use a dict[str, str]. This adds a new field to
Hyperparameter so that we can convert the datatypes at runtime.
instead of validating with isinstance(), we cast the hp value to the type it
is meant to be. This enforces a "strongly typed" value. When we
deserialize from the API string responses it becomes easier to deal with
too.
* Add wrapper for LDA. (aws#56)
Update CHANGELOG and bump the version number.
* Add support for async fit() (aws#59)
when calling fit(wait=False) it will return immediately. The training
job will carry on even if the process exits. by using attach() the
estimator can be retrieved by providing the training job name.
_prepare_init_params_from_job_description() is now a classmethod instead
of being a static method. Each class is responsible to implement their
specific logic to convert a training job description into arguments that
can be passed to its own __init__()
* Fix Estimator role expansion (aws#68)
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
* Add FM and LDA to the documentation. (aws#66)
* Fix description of an argument of sagemaker.session.train (aws#69)
* Fix description of an argument of sagemaker.session.train
'input_config' should be an array which has channel objects.
* Add a link to the botocore docs
* Use 'list' instead of 'array' in the description
* Add ntm algorithm with doc, unit tests, integ tests (aws#73)
* JSON serializer: predictor.predict accepts dictionaries (aws#62)
Add support for serializing python dictionaries to json
Add prediction with dictionary in tf iris integ test
* Fixing timeouts for PCA async integration test. (aws#78)
Execute tf_cifar test without logs to eliminate delay to detect that job has finished.
* Fixes in LinearLearner and unit tests addition. (aws#77)
* Print out billable seconds after training completes (aws#30)
* Added: print out billable seconds after training completes
* Fixed: test_session.py to pass unit tests
* Fixed: removed offending tzlocal()
* Use sagemaker_timestamp when creating endpoint names in integration tests. (aws#81)
* Support TensorFlow-1.5.0 and MXNet-1.0.0 (aws#82)
* Update .gitignore to ignore pytest_cache.
* Support TensorFlow-1.5.0 and MXNet-1.0.0
* Update and refactor tests. Add tests for fw_utils.
* Fix typo.
* Update changelog for 1.1.0 (aws#85)
@iquintero
iquintero deleted the service_role branch April 2, 2018 00:05
apacker pushed a commit to apacker/sagemaker-python-sdk that referenced this pull request Nov 15, 2018
Updated: XGBoost direct marketing notebook markdown at the end
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
Sign up for freeto 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.

creating estimator with role name fails if role is default sagemaker service role

2 participants

@iquintero@mvsusp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Fix Estimator role expansion - #68

Merged
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role
Feb 2, 2018
Merged

Fix Estimator role expansion#68
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role

Conversation

@iquintero

Copy link
Copy Markdown
Contributor

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.

This fixes#43

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
@iquintero
iquintero requested a review from owen-tFebruary 1, 2018 18:57

@mvsuspmvsusp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iquintero, can we write a unit test for that?

@iquintero

Copy link
Copy Markdown
ContributorAuthor

It doesn't really make sense to have a unit test for this, as I would have to mock the boto call to IAM and at that point there is nothing else left to test. Also the ARN is not being manually crafted anymore so we should be able to trust that boto is doing the right thing here.

@mvsusp

Copy link
Copy Markdown
Contributor

Agreed.

@iquintero
iquintero merged commit b45d79c into aws:masterFeb 2, 2018
jalabort added a commit to hudl/sagemaker-python-sdk that referenced this pull request Mar 1, 2018
* Add data_type to hyperparameters (aws#54)
When we describe a training job the data type of the hyper parameters is
lost because we use a dict[str, str]. This adds a new field to
Hyperparameter so that we can convert the datatypes at runtime.
instead of validating with isinstance(), we cast the hp value to the type it
is meant to be. This enforces a "strongly typed" value. When we
deserialize from the API string responses it becomes easier to deal with
too.
* Add wrapper for LDA. (aws#56)
Update CHANGELOG and bump the version number.
* Add support for async fit() (aws#59)
when calling fit(wait=False) it will return immediately. The training
job will carry on even if the process exits. by using attach() the
estimator can be retrieved by providing the training job name.
_prepare_init_params_from_job_description() is now a classmethod instead
of being a static method. Each class is responsible to implement their
specific logic to convert a training job description into arguments that
can be passed to its own __init__()
* Fix Estimator role expansion (aws#68)
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
* Add FM and LDA to the documentation. (aws#66)
* Fix description of an argument of sagemaker.session.train (aws#69)
* Fix description of an argument of sagemaker.session.train
'input_config' should be an array which has channel objects.
* Add a link to the botocore docs
* Use 'list' instead of 'array' in the description
* Add ntm algorithm with doc, unit tests, integ tests (aws#73)
* JSON serializer: predictor.predict accepts dictionaries (aws#62)
Add support for serializing python dictionaries to json
Add prediction with dictionary in tf iris integ test
* Fixing timeouts for PCA async integration test. (aws#78)
Execute tf_cifar test without logs to eliminate delay to detect that job has finished.
* Fixes in LinearLearner and unit tests addition. (aws#77)
* Print out billable seconds after training completes (aws#30)
* Added: print out billable seconds after training completes
* Fixed: test_session.py to pass unit tests
* Fixed: removed offending tzlocal()
* Use sagemaker_timestamp when creating endpoint names in integration tests. (aws#81)
* Support TensorFlow-1.5.0 and MXNet-1.0.0 (aws#82)
* Update .gitignore to ignore pytest_cache.
* Support TensorFlow-1.5.0 and MXNet-1.0.0
* Update and refactor tests. Add tests for fw_utils.
* Fix typo.
* Update changelog for 1.1.0 (aws#85)
@iquintero
iquintero deleted the service_role branch April 2, 2018 00:05
apacker pushed a commit to apacker/sagemaker-python-sdk that referenced this pull request Nov 15, 2018
Updated: XGBoost direct marketing notebook markdown at the end
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
Sign up for freeto 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.

creating estimator with role name fails if role is default sagemaker service role

2 participants

@iquintero@mvsusp
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Fix Estimator role expansion - #68

Merged
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role
Feb 2, 2018
Merged

Fix Estimator role expansion#68
iquintero merged 1 commit into
aws:masterfrom
iquintero:service_role

Conversation

@iquintero

Copy link
Copy Markdown
Contributor

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.

This fixes#43

Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
@iquintero
iquintero requested a review from owen-tFebruary 1, 2018 18:57

@mvsuspmvsusp left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@iquintero, can we write a unit test for that?

@iquintero

Copy link
Copy Markdown
ContributorAuthor

It doesn't really make sense to have a unit test for this, as I would have to mock the boto call to IAM and at that point there is nothing else left to test. Also the ARN is not being manually crafted anymore so we should be able to trust that boto is doing the right thing here.

@mvsusp

Copy link
Copy Markdown
Contributor

Agreed.

@iquintero
iquintero merged commit b45d79c into aws:masterFeb 2, 2018
jalabort added a commit to hudl/sagemaker-python-sdk that referenced this pull request Mar 1, 2018
* Add data_type to hyperparameters (aws#54)
When we describe a training job the data type of the hyper parameters is
lost because we use a dict[str, str]. This adds a new field to
Hyperparameter so that we can convert the datatypes at runtime.
instead of validating with isinstance(), we cast the hp value to the type it
is meant to be. This enforces a "strongly typed" value. When we
deserialize from the API string responses it becomes easier to deal with
too.
* Add wrapper for LDA. (aws#56)
Update CHANGELOG and bump the version number.
* Add support for async fit() (aws#59)
when calling fit(wait=False) it will return immediately. The training
job will carry on even if the process exits. by using attach() the
estimator can be retrieved by providing the training job name.
_prepare_init_params_from_job_description() is now a classmethod instead
of being a static method. Each class is responsible to implement their
specific logic to convert a training job description into arguments that
can be passed to its own __init__()
* Fix Estimator role expansion (aws#68)
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
* Add FM and LDA to the documentation. (aws#66)
* Fix description of an argument of sagemaker.session.train (aws#69)
* Fix description of an argument of sagemaker.session.train
'input_config' should be an array which has channel objects.
* Add a link to the botocore docs
* Use 'list' instead of 'array' in the description
* Add ntm algorithm with doc, unit tests, integ tests (aws#73)
* JSON serializer: predictor.predict accepts dictionaries (aws#62)
Add support for serializing python dictionaries to json
Add prediction with dictionary in tf iris integ test
* Fixing timeouts for PCA async integration test. (aws#78)
Execute tf_cifar test without logs to eliminate delay to detect that job has finished.
* Fixes in LinearLearner and unit tests addition. (aws#77)
* Print out billable seconds after training completes (aws#30)
* Added: print out billable seconds after training completes
* Fixed: test_session.py to pass unit tests
* Fixed: removed offending tzlocal()
* Use sagemaker_timestamp when creating endpoint names in integration tests. (aws#81)
* Support TensorFlow-1.5.0 and MXNet-1.0.0 (aws#82)
* Update .gitignore to ignore pytest_cache.
* Support TensorFlow-1.5.0 and MXNet-1.0.0
* Update and refactor tests. Add tests for fw_utils.
* Fix typo.
* Update changelog for 1.1.0 (aws#85)
@iquintero
iquintero deleted the service_role branch April 2, 2018 00:05
apacker pushed a commit to apacker/sagemaker-python-sdk that referenced this pull request Nov 15, 2018
Updated: XGBoost direct marketing notebook markdown at the end
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
Instead of manually constructing the role ARN, use the IAM boto client
to do it. This properly expands service-roles and regular roles.
Sign up for freeto 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.

creating estimator with role name fails if role is default sagemaker service role

2 participants

@iquintero@mvsusp